Canto Identity Subprotocols contest - 0xSmartContract's results

Summary

Issues List

Total 14 issues

[L-01] Protect your NFT from copying in fork

Occasionally, forks can happen on different blockchains.

There may be forked versions of Blockchains, which could cause confusion and lead to scams as duplicated NFT assets enter the market, then it could result in duplication of non-fungible tokens (NFTs) and there's likely to be some level of confusion around which assets are 'official' or 'authentic.’

A forked Blockchains, chain will thus have duplicated deeds that point to the same tokenURI

About Replay Attack: https://twitter.com/elerium115/status/1558471934924431363?s=20&t=RRheaYJwo-GmSnePwofgag

The most important part here is NFT's tokenURI detail. If the update I suggested below is not done, Duplicate NFTs will appear as a result, potentially leading to confusion and scams.

Tools Used

Manual Code Review

Recommended Mitigation Steps

src/CidNFT.sol:
  135      /// @return tokenURI The URI of the queried token (path to a JSON file)
  136:     function tokenURI(uint256 _id) public view override returns (string memory) {
+ 	 if (block.chainid != 1) revert wrongChain(); // Ethereum Chain ID : 1  - Polygon Chain ID : 137
  137:         if (ownerOf[_id] == address(0))
  138:             // According to ERC721, this revert for non-existing tokens is required
  139:             revert TokenNotMinted(_id);
  140:         return string(abi.encodePacked(baseURI, _id, ".json"));
  141:     }

4 results - 4 files

canto-bio-protocol/src/Bio.sol:
  42      /// @param _id ID to query for
  43:     function tokenURI(uint256 _id) public view override returns (string memory) {
  44          if (_ownerOf[_id] == address(0)) revert TokenNotMinted(_id);
+ 	 if (block.chainid != 7700) revert wrongChain(); // Canto Chain 7700


canto-namespace-protocol/src/Namespace.sol:
  89      /// @param _id ID to query for
  90:     function tokenURI(uint256 _id) public view override returns (string memory) {
  91          if (_ownerOf[_id] == address(0)) revert TokenNotMinted(_id);
+ 	 if (block.chainid != 7700) revert wrongChain(); // Canto Chain 7700


canto-namespace-protocol/src/Tray.sol:
  118      /// @param _id ID to query for
  119:     function tokenURI(uint256 _id) public view override returns (string memory) {
  120          if (!_exists(_id)) revert TrayNotMinted(_id);
+ 	 if (block.chainid != 7700) revert wrongChain(); // Canto Chain 7700


canto-pfp-protocol/src/ProfilePicture.sol:
  69      /// @dev Reverts if PFP is no longer owned by owner of associated CID NFT
  70:     function tokenURI(uint256 _id) public view override returns (string memory) {
  71          (address nftContract, uint256 nftID) = getPFP(_id);
+ 	 if (block.chainid != 7700) revert wrongChain(); // Canto Chain 7700

[L-02] No Check if OnErc721Received is implemented

Bio.mint() and ProfilePicture.mint()will mint a NFT . When minting a NFT, the function does not check if a receiving contract implements onERC721Received(). msg.sender can be contract address.

The intention behind this function is to check if the address receiving the NFT, if it is a contract, implements onERC721Received(). Thus, there is no check whether the receiving address supports ERC-721 tokens and position could be not transferrable in some cases.

Following shows that _mint is used instead of _safeMint

canto-bio-protocol/src/Bio.sol:
  120      /// @param _bio The text to add
  121:     function mint(string calldata _bio) external {
  122:         // We check the length in bytes, so will be higher for UTF-8 characters. But sufficient for this check
  123:         if (bytes(_bio).length == 0 || bytes(_bio).length > 200) revert InvalidBioLength(bytes(_bio).length);
  124:         uint256 tokenId = ++numMinted;
  125:         bio[tokenId] = _bio;
  126:         _mint(msg.sender, tokenId);
  127:         emit BioAdded(msg.sender, tokenId, _bio);
  128:     }
  129  }


canto-pfp-protocol/src/ProfilePicture.sol:
  78      /// @param _nftID The nft ID to reference
  79:     function mint(address _nftContract, uint256 _nftID) external {
  80:         uint256 tokenId = ++numMinted;
  81:         if (ERC721(_nftContract).ownerOf(_nftID) != msg.sender)
  82:             revert PFPNotOwnedByCaller(msg.sender, _nftContract, _nftID);
  83:         ProfilePictureData storage pictureData = pfp[tokenId];
  84:         pictureData.nftContract = _nftContract;
  85:         pictureData.nftID = _nftID;
  86:         _mint(msg.sender, tokenId);
  87:         emit PfpAdded(msg.sender, tokenId, _nftContract, _nftID);
  88:     }

Recommendation Consider using _safeMint instead of _mint

[L-03] Project Upgrade and Stop Scenario should be

At the start of the project, the system may need to be stopped or upgraded, I suggest you have a script beforehand and add it to the documentation. This can also be called an " EMERGENCY STOP (CIRCUIT BREAKER) PATTERN ".

https://github.com/maxwoe/solidity_patterns/blob/master/security/EmergencyStop.sol

[L-04] Add to Event-Emit for constructors

Monitoring the amount of Ether coming to the contract outside the chain is an important initialize data

4 results

canto-bio-protocol/src/Bio.sol:
  31      /// @notice Initiates CSR on mainnet
  32:     constructor() ERC721("Biography", "Bio") {
  33:         if (block.chainid == 7700) {
  34:             // Register CSR on Canto mainnnet
  35:             Turnstile turnstile = Turnstile(0xEcf044C5B4b867CFda001101c617eCd347095B44);
  36:             turnstile.register(tx.origin);
  37:         }
  38:     }

canto-namespace-protocol/src/Namespace.sol:
  72      /// @param _revenueAddress Adress to send the revenue to
  73:     constructor(
  74:         address _tray,
  75:         address _note,
  76:         address _revenueAddress
  77:     ) ERC721("Namespace", "NS") Owned(msg.sender) {
  78:         tray = Tray(_tray);
  79:         note = ERC20(_note);
  80:         revenueAddress = _revenueAddress;
  81:         if (block.chainid == 7700) {
  82:             // Register CSR on Canto mainnnet
  83:             Turnstile turnstile = Turnstile(0xEcf044C5B4b867CFda001101c617eCd347095B44);
  84:             turnstile.register(tx.origin);
  85:         }
  86:     }



canto-namespace-protocol/src/Tray.sol:
   97      /// @param _namespaceNFT Address of the Namespace NFT
   98:     constructor(
   99:         bytes32 _initHash,
  100:         uint256 _trayPrice,
  101:         address _revenueAddress,
  102:         address _note,
  103:         address _namespaceNFT
  104:     ) ERC721A("Namespace Tray", "NSTRAY") Owned(msg.sender) {
  105:         lastHash = _initHash;
  106:         trayPrice = _trayPrice;
  107:         revenueAddress = _revenueAddress;
  108:         note = ERC20(_note);
  109:         namespaceNFT = _namespaceNFT;
  110:         if (block.chainid == 7700) {
  111:             // Register CSR on Canto mainnnet
  112:             Turnstile turnstile = Turnstile(0xEcf044C5B4b867CFda001101c617eCd347095B44);
  113:             turnstile.register(tx.origin);
  114:         }
  115:     }

canto-pfp-protocol/src/ProfilePicture.sol:
  56      /// @param _subprotocolName Name with which the subprotocol is / will be registered in the registry. Registration will not be performed automatically
  57:     constructor(address _cidNFT, string memory _subprotocolName) ERC721("Profile Picture", "PFP") {
  58:         cidNFT = ICidNFT(_cidNFT);
  59:         subprotocolName = _subprotocolName;
  60:         if (block.chainid == 7700) {
  61:             // Register CSR on Canto mainnnet
  62:             Turnstile turnstile = Turnstile(0xEcf044C5B4b867CFda001101c617eCd347095B44);
  63:             turnstile.register(tx.origin);
  64:         }
  65:     }

[L-05] Critical Address Changes Should Use Check Pattern

From time to time, 0 addresses or wrong token addresses can be identified by mistake

canto-namespace-protocol/src/Namespace.sol:
  195      /// @param _newNoteAddress New address to use
  196:     function changeNoteAddress(address _newNoteAddress) external onlyOwner {
  197:         address currentNoteAddress = address(note);
  198:         note = ERC20(_newNoteAddress);
  199:         emit NoteAddressUpdate(currentNoteAddress, _newNoteAddress);
  200:     }
  201  

canto-namespace-protocol/src/Tray.sol:
  209      /// @param _newNoteAddress New address to use
  210:     function changeNoteAddress(address _newNoteAddress) external onlyOwner {
  211:         address currentNoteAddress = address(note);
  212:         note = ERC20(_newNoteAddress);
  213:         emit NoteAddressUpdate(currentNoteAddress, _newNoteAddress);
  214:     }

[L-06] Solmate's SafeTransferLib doesn't check whether the ERC20 contract exists

Solmate's SafeTransferLib, which is often used to interact with non-compliant/unsafe ERC20 tokens, does not check whether the ERC20 contract exists. The following code will not revert in case the token doesn't exist (yet).

This is stated in the Solmate library: https://github.com/transmissions11/solmate/blob/main/src/utils/SafeTransferLib.sol#L9

2 results

canto-namespace-protocol/src/Namespace.sol:
  114:         SafeTransferLib.safeTransferFrom(note, msg.sender, revenueAddress, fusingCosts);

canto-namespace-protocol/src/Tray.sol:

  157:          SafeTransferLib.safeTransferFrom(note, msg.sender, revenueAddress, _amount * trayPrice);

https://twitter.com/zarfsec/status/1595070426782535681?t=Hu-p5n6afaQS6RvdfClwAg&s=19

Recommended Mitigation Steps

Add a contract exist control in functions;

pragma solidity >=0.8.0;

function isContract(address _addr) private returns (bool isContract) {
    isContract = _addr.code.length > 0;
}

[N-07] Add EIP-2981 NFT Royalty Standart Support

Consider adding EIP-2981 NFT Royalty Standard to the project

https://eips.ethereum.org/EIPS/eip-2981

Royalty (Copyright – EIP 2981):

Fixed % royalties: For example, 6% of all sales go back to artists Declining royalties: There may be a continuous decline in sales based on time or any other variable. Dynamic royalties : Varies over time or sales amount Upgradeable royalties: Allows a legal entity or NFT owner to change any copyright Incremental royalties: No royalties, for example when sold for less than $100 Managed royalties : Funds are owned by a DAO, imagine the recipient is a DAO treasury Royalties to different people : Collectors and artists can even use royalties, not specific to a particular personality

[N-08] Use SMTChecker

The highest tier of smart contract behavior assurance is formal mathematical verification. All assertions that are made are guaranteed to be true across all inputs → The quality of your asserts is the quality of your verification.

https://twitter.com/0xOwenThurm/status/1614359896350425088?t=dbG9gHFigBX85Rv29lOjIQ&s=19

[N-09] Constants on the left are better

If you use the constant first you support structures that veil programming errors. And one should only produce code either to add functionality, fix an programming error or trying to support structures to avoid programming errors (like design patterns).

https://www.moserware.com/2008/01/constants-on-left-are-better-but-this.html

3 results - 2 files

canto-bio-protocol/src/Bio.sol:
  123:         if (bytes(_bio).length == 0 || bytes(_bio).length > 200) revert InvalidBioLength(bytes(_bio).length);
  160:         if (bytes(_bio).length == 0 || bytes(_bio).length > 200) revert InvalidBioLength(bytes(_bio).length);

canto-pfp-protocol/src/ProfilePicture.sol:
  101:         if (cidNFTID == 0 || addressRegistry.getAddress(cidNFTID) != ERC721(nftContract).ownerOf(nftID)) {

[N-10] Function Calls in Loop Could Lead to Denial of Service due to Array length not being checked

2 results - 2 files:

canto-namespace-protocol\src\Namespace.sol:

  109      /// @param _characterList The tiles to use for the fusing
  110:     function fuse(CharacterData[] calldata _characterList) external {
+              require(_characterList.length < max _characterListLengt, "max length");

  111:         uint256 numCharacters = _characterList.length;
  112:         if (numCharacters > 13 || numCharacters == 0) revert InvalidNumberOfCharacters(numCharacters);
  113:         uint256 fusingCosts = 2**(13 - numCharacters) * 1e18;
  114:         SafeTransferLib.safeTransferFrom(note, msg.sender, revenueAddress, fusingCosts);
  115:         uint256 namespaceIDToMint = ++nextNamespaceIDToMint;
  116:         Tray.TileData[] storage nftToMintCharacters = nftCharacters[namespaceIDToMint];
  117:         bytes memory bName = new bytes(numCharacters * 33); // Used to convert into a string. Can be 33 times longer than the string at most (longest zalgo characters is 33 bytes)
  118:         uint256 numBytes;
  119:         // Extract unique trays for burning them later on
  120:         uint256 numUniqueTrays;
  121:         uint256[] memory uniqueTrays = new uint256[](_characterList.length);
  122:         for (uint256 i; i < numCharacters; ++i) {
  123:             bool isLastTrayEntry = true;
  124:             uint256 trayID = _characterList[i].trayID;
  125:             uint8 tileOffset = _characterList[i].tileOffset;
  126:             // Check for duplicate characters in the provided list. 1/2 * n^2 loop iterations, but n is bounded to 13 and we do not perform any storage operations

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Namespace.sol#L110

canto-namespace-protocol\src\Utils.sol:

  222:     function generateSVG(Tray.TileData[] memory _tiles, bool _isTray) internal pure returns (string memory) {
+              require(_tiles.length < max _tilesLengt, "max length");
  223:         string memory innerData;
  224:         uint256 dx = 400 / (_tiles.length + 1);
  225:         for (uint256 i; i < _tiles.length; ++i) {
  226:             innerData = string.concat(
  227:                 innerData,
  228:                 '<text dominant-baseline="middle" text-anchor="middle" y="100" x="',
  229:                 LibString.toString(dx * (i + 1)),
  230:                 '">',
  231:                 string(
  232:                     characterToUnicodeBytes(_tiles[i].fontClass, _tiles[i].characterIndex, _tiles[i].characterModifier)
  233:                 ),
  234:                 "</text>"
  235:             );
  236:             if (_isTray) {
  237:                 innerData = string.concat(
  238:                     innerData,
  239:                     '<rect width="34" height="60" y="70" x="',
  240:                     LibString.toString(dx * (i + 1) - 17),
  241:                     '" stroke="black" stroke-width="1" fill="none"></rect>'
  242:                 );
  243:             }
  244:         }
  245:         return
  246:             string.concat(
  247:                 '<svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMinYMin meet" viewBox="0 0 400 200"><style>text { font-family: sans-serif; font-size: 30px; }</style>',
  248:                 innerData,
  249:                 "</svg>"
  250:             );
  251:     }

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Utils.sol#L222

Recommendation: Function calls made in unbounded loop are error-prone with potential resource exhaustion as it can trap the contract due to gas limitations or failed transactions. Consider bounding the loop length if the array is expected to be growing and/or handling a huge list of elements to avoid unnecessary gas wastage and denial of service.

[N-11] Take advantage of Custom Error's return value property

An important feature of Custom Error is that values such as address, tokenID, msg.value can be written inside the () sign, this kind of approach provides a serious advantage in debugging and examining the revert details of dapps such as tenderly.

For Example;

3 results - 1 file

canto-namespace-protocol/src/Namespace.sol:
  137:                 revert CannotFuseCharacterWithSkinTone();
  161:                 ) revert CallerNotAllowedToFuse();
  187:             revert CallerNotAllowedToBurn();

[N-12] Function writing that does not comply with the Solidity Style Guide

Context: All Contracts

Description: Order of Functions; ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier.But there are contracts in the project that do not comply with this.

https://docs.soliditylang.org/en/v0.8.17/style-guide.html

Functions should be grouped according to their visibility and ordered:

constructor receive function (if exists) fallback function (if exists) external public internal private within a grouping, place the view and pure functions last

[N-13] Tokens accidentally sent to the contract cannot be recovered

Contex canto-bio-protocol/src/Bio.sol

It can't be recovered if the tokens accidentally arrive at the contract address, which has happened to many popular projects, so I recommend adding a recovery code to your critical contracts.

Recommended Mitigation Steps

Add this code:

 /**
  * @notice Sends ERC20 tokens trapped in contract to external address
  * @dev Onlyowner is allowed to make this function call
  * @param account is the receiving address
  * @param externalToken is the token being sent
  * @param amount is the quantity being sent
  * @return boolean value indicating whether the operation succeeded.
  *
 */
  function rescueERC20(address account, address externalToken, uint256 amount) public onlyOwner returns (bool) {
    IERC20(externalToken).transfer(account, amount);
    return true;
  }
}

[N-14] Use a more recent version of Solidity

Context: All contracts

Description: For security, it is best practice to use the latest Solidity version. For the security fix list in the versions; https://github.com/ethereum/solidity/blob/develop/Changelog.md

Recommendation: Old version of Solidity is used (0.8.0), newer version can be used (0.8.17)

Summary

Gas Optimizations List

[G-01] Gas overflow during iteration (DoS)

Each iteration of the cycle requires a gas flow. A moment may come when more gas is required than it is allocated to record one block. In this case, all iterations of the loop will fail.

2 results - 2 files:

canto-namespace-protocol\src\Namespace.sol:

  109      /// @param _characterList The tiles to use for the fusing
  110:     function fuse(CharacterData[] calldata _characterList) external {
+              require(_characterList.length < max _characterListLengt, "max length");

  111:         uint256 numCharacters = _characterList.length;
  112:         if (numCharacters > 13 || numCharacters == 0) revert InvalidNumberOfCharacters(numCharacters);
  113:         uint256 fusingCosts = 2**(13 - numCharacters) * 1e18;
  114:         SafeTransferLib.safeTransferFrom(note, msg.sender, revenueAddress, fusingCosts);
  115:         uint256 namespaceIDToMint = ++nextNamespaceIDToMint;
  116:         Tray.TileData[] storage nftToMintCharacters = nftCharacters[namespaceIDToMint];
  117:         bytes memory bName = new bytes(numCharacters * 33); // Used to convert into a string. Can be 33 times longer than the string at most (longest zalgo characters is 33 bytes)
  118:         uint256 numBytes;
  119:         // Extract unique trays for burning them later on
  120:         uint256 numUniqueTrays;
  121:         uint256[] memory uniqueTrays = new uint256[](_characterList.length);
  122:         for (uint256 i; i < numCharacters; ++i) {
  123:             bool isLastTrayEntry = true;
  124:             uint256 trayID = _characterList[i].trayID;
  125:             uint8 tileOffset = _characterList[i].tileOffset;
  126:             // Check for duplicate characters in the provided list. 1/2 * n^2 loop iterations, but n is bounded to 13 and we do not perform any storage operations

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Namespace.sol#L110

canto-namespace-protocol\src\Utils.sol:

  222:     function generateSVG(Tray.TileData[] memory _tiles, bool _isTray) internal pure returns (string memory) {
+              require(_tiles.length < max _tilesLengt, "max length");
  223:         string memory innerData;
  224:         uint256 dx = 400 / (_tiles.length + 1);
  225:         for (uint256 i; i < _tiles.length; ++i) {
  226:             innerData = string.concat(
  227:                 innerData,
  228:                 '<text dominant-baseline="middle" text-anchor="middle" y="100" x="',
  229:                 LibString.toString(dx * (i + 1)),
  230:                 '">',
  231:                 string(
  232:                     characterToUnicodeBytes(_tiles[i].fontClass, _tiles[i].characterIndex, _tiles[i].characterModifier)
  233:                 ),
  234:                 "</text>"
  235:             );
  236:             if (_isTray) {
  237:                 innerData = string.concat(
  238:                     innerData,
  239:                     '<rect width="34" height="60" y="70" x="',
  240:                     LibString.toString(dx * (i + 1) - 17),
  241:                     '" stroke="black" stroke-width="1" fill="none"></rect>'
  242:                 );
  243:             }
  244:         }
  245:         return
  246:             string.concat(
  247:                 '<svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMinYMin meet" viewBox="0 0 400 200"><style>text { font-family: sans-serif; font-size: 30px; }</style>',
  248:                 innerData,
  249:                 "</svg>"
  250:             );
  251:     }

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Utils.sol#L222

Here are the data available in the covered contracts. The use of this situation in contracts that are not covered will also provide gas optimization.

[G-02] State variables only set in the constructor should be declared immutable

Avoids a Gsset (20000 gas) in the constructor and replaces the first access in each transaction (Gcoldsload - 2100 gas) and each access thereafter (Gwarmacces - 100 gas) with a PUSH32 (3 gas).

canto-pfp-protocol\src\ProfilePicture.sol:

  35:     string public subprotocolName;

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-pfp-protocol/src/ProfilePicture.sol#L35

[G-03] Failure to check the zero address in the constructor causes the contract to be deployed again

Zero address control is not performed in the constructor in all 3 contracts within the scope of the audit. Bypassing this check could cause the contract to be deployed by mistakenly entering a zero address. In this case, the contract will need to be redeployed. This means extra gas consumption as contract deployment costs are high.

3 results - 3 files:

canto-namespace-protocol\src\Namespace.sol:

  73:     constructor(
  74:         address _tray,
  75:         address _note,
  76:         address _revenueAddress
  77:     ) ERC721("Namespace", "NS") Owned(msg.sender) {
  78:         tray = Tray(_tray);
  79:         note = ERC20(_note);
  80:         revenueAddress = _revenueAddress;
  81:         if (block.chainid == 7700) {
  82:             // Register CSR on Canto mainnnet
  83:             Turnstile turnstile = Turnstile(0xEcf044C5B4b867CFda001101c617eCd347095B44);
  84:             turnstile.register(tx.origin);
  85:         }
  86:     }

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Namespace.sol#L73

canto-namespace-protocol\src\Tray.sol:

   97      /// @param _namespaceNFT Address of the Namespace NFT
   98:     constructor(
   99:         bytes32 _initHash,
  100:         uint256 _trayPrice,
  101:         address _revenueAddress,
  102:         address _note,
  103:         address _namespaceNFT
  104:     ) ERC721A("Namespace Tray", "NSTRAY") Owned(msg.sender) {
  105:         lastHash = _initHash;
  106:         trayPrice = _trayPrice;
  107:         revenueAddress = _revenueAddress;
  108:         note = ERC20(_note);
  109:         namespaceNFT = _namespaceNFT;
  110:         if (block.chainid == 7700) {
  111:             // Register CSR on Canto mainnnet
  112:             Turnstile turnstile = Turnstile(0xEcf044C5B4b867CFda001101c617eCd347095B44);
  113:             turnstile.register(tx.origin);
  114:         }
  115:     }

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Tray.sol#L98

canto-pfp-protocol\src\ProfilePicture.sol:

  56      /// @param _subprotocolName Name with which the subprotocol is / will be registered in the registry. Registration will not be performed automatically
  57:     constructor(address _cidNFT, string memory _subprotocolName) ERC721("Profile Picture", "PFP") {
  58:         cidNFT = ICidNFT(_cidNFT);
  59:         subprotocolName = _subprotocolName;
  60:         if (block.chainid == 7700) {
  61:             // Register CSR on Canto mainnnet
  62:             Turnstile turnstile = Turnstile(0xEcf044C5B4b867CFda001101c617eCd347095B44);
  63:             turnstile.register(tx.origin);
  64:         }
  65:     }

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-pfp-protocol/src/ProfilePicture.sol#L57

[G-04] By updating emit, extra emit usage can be reduced

In the mint function in the Bio.sol contract, the protocol emits two emits (the _mint and bioAdded emits). By updating the emit of Solady's _mint function, the use of one extra emit can be reduced.

Reference: https://github.com/ethereum/go-ethereum/blob/master/params/protocol_params.go#L78-L82

canto-bio-protocol/lib/solady/lib/solmate/src/tokens/ERC721.sol:

+ event Transfer(address from, address indexed to, uint256 indexed id, string indexed bio);

+ 157:     function _mint(address to, uint256 id) internal virtual {
+ 158:         require(to != address(0), "INVALID_RECIPIENT");
+ 159: 
+ 160:         require(_ownerOf[id] == address(0), "ALREADY_MINTED");
+ 161: 
+ 162:         // Counter overflow is incredibly unrealistic.
+ 163:         unchecked {
+ 164:             _balanceOf[to]++;
+ 165:         }
+ 166: 
+ 167:         _ownerOf[id] = to;
+ 168: 
+ 169:         emit Transfer(address(0), to, tokenId, _bio );
+ 170:     }


canto-bio-protocol/src/Bio.sol:

23:     event BioAdded(address indexed minter, uint256 indexed nftID, string indexed bio);
  120      /// @param _bio The text to add
  121:     function mint(string calldata _bio) external {
  122:         // We check the length in bytes, so will be higher for UTF-8 characters. But sufficient for this check
  123:         if (bytes(_bio).length == 0 || bytes(_bio).length > 200) revert InvalidBioLength(bytes(_bio).length);
  124:         uint256 tokenId = ++numMinted;
  125:         bio[tokenId] = _bio;
- 126:        _mint(msg.sender, tokenId,);
+ 126:        _mint(msg.sender, tokenId, _bio);

- 127:        emit BioAdded(msg.sender, tokenId, _bio);
  128:     }

[G-05] Emit can be rearranged

The emit can be rearranged in the changeNoteAddress() and changeRevenueAddress() functions. With this arrangement, both the deploy time (~1.4 k * 4 = 5.6k gas per function) and every time the functions are triggered (~16 gas) gas savings will be achieved.

4 results - 2 files:

canto-namespace-protocol\src\Tray.sol:

  209      /// @param _newNoteAddress New address to use
  210:     function changeNoteAddress(address _newNoteAddress) external onlyOwner {
  211:         address currentNoteAddress = address(note);
  212:         note = ERC20(_newNoteAddress);
  213:         emit NoteAddressUpdate(currentNoteAddress, _newNoteAddress);
  214:     }
  

  217      /// @param _newRevenueAddress New address to use
  218:     function changeRevenueAddress(address _newRevenueAddress) external onlyOwner {
  219:         address currentRevenueAddress = revenueAddress;
  220:         revenueAddress = _newRevenueAddress;
  221:         emit RevenueAddressUpdated(currentRevenueAddress, _newRevenueAddress);
  222:     }

canto-namespace-protocol\src\Namespace.sol:

  195      /// @param _newNoteAddress New address to use
  196:     function changeNoteAddress(address _newNoteAddress) external onlyOwner {
  197:         address currentNoteAddress = address(note);
  198:         note = ERC20(_newNoteAddress);
  199:         emit NoteAddressUpdate(currentNoteAddress, _newNoteAddress);
  200:     }


  203      /// @param _newRevenueAddress New address to use
  204:     function changeRevenueAddress(address _newRevenueAddress) external onlyOwner {
  205:         address currentRevenueAddress = revenueAddress;
  206:         revenueAddress = _newRevenueAddress;
  207:         emit RevenueAddressUpdated(currentRevenueAddress, _newRevenueAddress);
  208:     }
  209  }

canto-namespace-protocol\src\Namespace.sol:

  195      /// @param _newNoteAddress New address to use
  196:     function changeNoteAddress(address _newNoteAddress) external onlyOwner {
- 197:         address currentNoteAddress = address(note);
- 198:         note = ERC20(_newNoteAddress);
- 199:         emit NoteAddressUpdate(currentNoteAddress, _newNoteAddress);
+ 199:         emit NoteAddressUpdate(note, _newNoteAddress);
+ 198:         note = ERC20(_newNoteAddress);
  200:     }

[G-06] Use assembly to write address storage values

2 results - 4 files:

canto-namespace-protocol\src\Namespace.sol:

  73:     constructor(
  80:         revenueAddress = _revenueAddress;


  204:     function changeRevenueAddress(address _newRevenueAddress) external onlyOwner {
  206:         revenueAddress = _newRevenueAddress;

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Namespace.sol#L80

canto-namespace-protocol\src\Tray.sol:

   98:     constructor(
  107:         revenueAddress = _revenueAddress;


  218:     function changeRevenueAddress(address _newRevenueAddress) external onlyOwner {
  220:         revenueAddress = _newRevenueAddress;

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Tray.sol#L107

Recommendation Code:

canto-namespace-protocol\src\Tray.sol:

   98:     constructor(
- 107:         revenueAddress = _revenueAddress;
+                  assembly {                      
+                      sstore(vault. revenueAddress, _revenueAddress)
+                  }                               
          

[G-07] Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

6 results - 2 files:

canto-namespace-protocol\src\Namespace.sol:

  196:     function changeNoteAddress(address _newNoteAddress) external onlyOwner {

  204:     function changeRevenueAddress(address _newRevenueAddress) external onlyOwner {

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Namespace.sol#L196

canto-namespace-protocol\src\Tray.sol:

  210:     function changeNoteAddress(address _newNoteAddress) external onlyOwner {

  218:     function changeRevenueAddress(address _newRevenueAddress) external onlyOwner {

  225:     function endPrelaunchPhase() external onlyOwner {

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Tray.sol#L210

[G-08] Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.

2 results - 1 file:

canto-namespace-protocol\src\Namespace.sol:

  43:     mapping(string => uint256) public nameToToken;
  46:     mapping(uint256 => string) public tokenToName;

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Namespace.sol#L43-L46

[G-09] Use a more recent version of solidity

Use a solidity version of at least 0.8.0 to get overflow protection without SafeMath Use a solidity version of at least 0.8.2 to get simple compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value In 0.8.15 the conditions necessary for inlining are relaxed. Benchmarks show that the change significantly decreases the bytecode size (which impacts the deployment cost) while the effect on the runtime gas usage is smaller.

In 0.8.17 prevent the incorrect removal of storage writes before calls to Yul functions that conditionally terminate the external EVM call; Simplify the starting offset of zero-length operations to zero. More efficient overflow checks for multiplication.

5 results - 5 files:

canto-bio-protocol/src/Bio.sol:

  2: pragma solidity >=0.8.0;

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-bio-protocol/src/Bio.sol#L2

canto-namespace-protocol/src/Namespace.sol:
  
  2: pragma solidity >=0.8.0;

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Namespace.sol#L2

canto-namespace-protocol/src/Tray.sol:

  2: pragma solidity >=0.8.0;

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Tray.sol#L2

canto-namespace-protocol/src/Utils.sol:
  
  2: pragma solidity >=0.8.0;

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Utils.sol#L2

canto-pfp-protocol/src/ProfilePicture.sol:
  
  2: pragma solidity >=0.8.0;
  

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-pfp-protocol/src/ProfilePicture.sol#L2

[G-10] The result of function calls should be cached rather than re-calling the function

canto-namespace-protocol\src\Tray.sol:
 
  245:     function _drawing(uint256 _seed) private pure returns (TileData memory tileData) {

  247:         uint256 charRandValue = Utils.iteratePRNG(_seed); // Iterate PRNG to not have any biasedness / correlation between random numbers

  265:                     // Set seed for Zalgo to ensure same characters will be always generated for this tile
- 266:                     uint256 zalgoSeed = Utils.iteratePRNG(_seed);
+ 266:                     uint256 zalgoSeed = charRandValue;
  267:                     tileData.characterModifier = uint8(zalgoSeed % 256);
  268                  }

[G-11] Avoid contract existence checks by using low level calls

Prior to 0.8.10 the compiler inserted extra code, including EXTCODESIZE (100 gas), to check for contract existence for external function calls. In more recent solidity versions, the compiler will not insert these checks if the external call has a return value. Similar behavior can be achieved in earlier versions by using low-level calls, since low level calls never check for contract existence.

12 results - 4 files:

canto-bio-protocol\src\Bio.sol:

  // @audit register()
  36:    turnstile.register(tx.origin);

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-bio-protocol/src/Bio.sol#L36

canto-namespace-protocol\src\Namespace.sol:

  // @audit register() 
  84:    turnstile.register(tx.origin);

  // @audit getTile()
  133:   Tray.TileData memory tileData = tray.getTile(trayID, tileOffset); // Will revert if tileOffset is too high

  // @audit ownerOf()
  156:   address trayOwner = tray.ownerOf(trayID);

  // @audit getApproved()
  159:   tray.getApproved(trayID) != msg.sender &&
  
  // @audit isApprovedForAll()
  160:   !tray.isApprovedForAll(trayOwner, msg.sender)

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Namespace.sol#L84

canto-namespace-protocol\src\Tray.sol:

  // @audit register() 
  113:   turnstile.register(tx.origin);

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Tray.sol#L113

canto-pfp-protocol\src\ProfilePicture.sol:

  // @audit register()  
  63:    turnstile.register(tx.origin);

  // @audit tokenURI()
  73:    return ERC721(nftContract).tokenURI(nftID);

  // @audit ownerOf()
  81:    if (ERC721(_nftContract).ownerOf(_nftID) != msg.sender)

  // @audit addressRegistry()
  100:   IAddressRegistry addressRegistry = cidNFT.addressRegistry();

  // @audit getAddress(), ownerOf() 
  101:   if (cidNFTID == 0 || addressRegistry.getAddress(cidNFTID) != ERC721(nftContract).ownerOf(nftID)) {

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-pfp-protocol/src/ProfilePicture.sol#L63

[G-12] bytes constants are more eficient than string constans

If the data can fit in 32 bytes, the bytes32 data type can be used instead of bytes or strings, as it is less robust in terms of robustness.

canto-pfp-protocol\src\ProfilePicture.sol:
  
  35:     string public subprotocolName;

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-pfp-protocol/src/ProfilePicture.sol#L35

[G-13] Change public function visibility to external

Since the following public functions are not called from within the contract, their visibility can be made external for gas optimization.

3 results - 3 files:

canto-bio-protocol\src\Bio.sol:

  43:     function tokenURI(uint256 _id) public view override returns (string memory) {

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-bio-protocol/src/Bio.sol#L43

canto-namespace-protocol\src\Namespace.sol:

  90:     function tokenURI(uint256 _id) public view override returns (string memory) {

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Namespace.sol#L90

canto-namespace-protocol\src\Tray.sol:

  119:     function tokenURI(uint256 _id) public view override returns (string memory) {

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Tray.sol#L119

[G-14] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contracts gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html

Use a larger size then downcast where needed.

15 result - 2 files:

canto-namespace-protocol\src\Tray.sol:

  // @audit uint8 fontClass
  259:     tileData.fontClass = 3 + uint8((res - 80) / 8);
  261:     tileData.fontClass = 5 + uint8((res - 96) / 4);
  263:     tileData.fontClass = 7 + uint8((res - 104) / 2);


  // @audit uint8 characterModifier
  267:     tileData.characterModifier = uint8(zalgoSeed % 256);

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Tray.sol#L259

canto-namespace-protocol\src\Utils.sol:

   // @audit uint16 _characterIndex
   85:     byteOffset = _characterIndex * 3;
   86:     supportsSkinToneModifier = _characterIndex >= EMOJIS_LE_THREE_BYTES - EMOJIS_MOD_SKIN_TONE_THREE_BYTES;
   89:     byteOffset = EMOJIS_BYTE_OFFSET_FOUR_BYTES + (_characterIndex - EMOJIS_LE_THREE_BYTES) * 4;
   93:     byteOffset = EMOJIS_BYTE_OFFSET_SIX_BYTES + (_characterIndex - EMOJIS_LE_FOUR_BYTES) * 6;
   96:     byteOffset = EMOJIS_BYTE_OFFSET_SEVEN_BYTES + (_characterIndex - EMOJIS_LE_SIX_BYTES) * 7;
   99:     byteOffset = EMOJIS_BYTE_OFFSET_EIGHT_BYTES + (_characterIndex - EMOJIS_LE_SEVEN_BYTES) * 8;
  102:     byteOffset = EMOJIS_BYTE_OFFSET_FOURTEEN_BYTES + (_characterIndex - EMOJIS_LE_EIGHT_BYTES) * 14;

  // @audit uint8 asciiStartingIndex
  135:     return abi.encodePacked(bytes1(asciiStartingIndex + uint8(_characterIndex)));

  // @audit uint8 asciiStartingIndex, _characterIndex
  145:     bytes memory character = abi.encodePacked(bytes1(asciiStartingIndex + uint8(_characterIndex)));
  273:     uint8 lastByteSum = lastByteVal + _characterIndex;
  277:     lastByteVal = 128 + (lastByteSum % 192);

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Utils.sol#L85

[G-15] Setting the constructor to payable

You can cut out 10 opcodes in the creation-time EVM bytecode if you declare a constructor payable. Making the constructor payable eliminates the need for an initial check of msg.value == 0 and saves 13 gas on deployment with no security risks.

4 results - 4 files:

canto-bio-protocol\src\Bio.sol:

  32:     constructor() ERC721("Biography", "Bio") {

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-bio-protocol/src/Bio.sol#L32

canto-namespace-protocol\src\Namespace.sol:

  73:     constructor(

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Namespace.sol#L73

canto-namespace-protocol\src\Tray.sol:

  98:     constructor(

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Tray.sol#L98

canto-pfp-protocol\src\ProfilePicture.sol:

  57:     constructor(address _cidNFT, string memory _subprotocolName) ERC721("Profile Picture", "PFP") {

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-pfp-protocol/src/ProfilePicture.sol#L57

Recommendation: Set the constructor to payable

[G-16] Use nested if and, avoid multiple check combinations

Using nested is cheaper than using && multiple check combinations. There are more advantages, such as easier to read code and better coverage reports.

7 results - 3 files:

canto-bio-protocol\src\Bio.sol:

  60:     if ((i > 0 && (i + 1) % 40 == 0) || prevByteWasContinuation || i == lengthInBytes - 1) {

  65:     if (nextCharacter & 0xC0 == 0x80) {

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-bio-protocol/src/Bio.sol#L60

canto-namespace-protocol\src\Namespace.sol:

  136:    if (tileData.fontClass != 0 && _characterList[i].skinToneModifier != 0) {


  157:    if (
  158:        trayOwner != msg.sender &&
  159:        tray.getApproved(trayID) != msg.sender &&
  160:        !tray.isApprovedForAll(trayOwner, msg.sender)
  161:    ) revert CallerNotAllowedToFuse();


  186:    if (nftOwner != msg.sender && getApproved[_id] != msg.sender && !isApprovedForAll[nftOwner][msg.sender])

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Namespace.sol#L136

canto-namespace-protocol\src\Tray.sol:

  175:    if (
  176:        namespaceNFT != msg.sender &&
  177:        trayOwner != msg.sender &&
  178:        getApproved(_id) != msg.sender &&
  179:        !isApprovedForAll(trayOwner, msg.sender)
  180:    ) revert CallerNotAllowedToBurn();


  184:    if (numPrelaunchMinted != type(uint256).max && _id <= numPrelaunchMinted)

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Tray.sol#L175-LL180

[G-17] Sort Solidity operations using short-circuit mode

Short-circuiting is a solidity contract development model that uses OR/AND logic to sequence different cost operations. It puts low gas cost operations in the front and high gas cost operations in the back, so that if the front is low If the cost operation is feasible, you can skip (short-circuit) the subsequent high-cost Ethereum virtual machine operation.

//f(x) is a low gas cost operation 
//g(y) is a high gas cost operation 

//Sort operations with different gas costs as follows 
f(x) || g(y) 
f(x) && g(y)

4 results - 3 files:

canto-bio-protocol\src\Bio.sol:

  60:    if ((i > 0 && (i + 1) % 40 == 0) || prevByteWasContinuation || i == lengthInBytes - 1) {

 123:    if (bytes(_bio).length == 0 || bytes(_bio).length > 200) revert InvalidBioLength(bytes(_bio).length);

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-bio-protocol/src/Bio.sol#L60

canto-namespace-protocol\src\Namespace.sol:

  112:   if (numCharacters > 13 || numCharacters == 0) revert InvalidNumberOfCharacters(numCharacters);

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Namespace.sol#L112

canto-pfp-protocol\src\ProfilePicture.sol:

  101:   if (cidNFTID == 0 || addressRegistry.getAddress(cidNFTID) != ERC721(nftContract).ownerOf(nftID)) {

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-pfp-protocol/src/ProfilePicture.sol#L101

[G-18] Use ERC721A instead ERC721

ERC721A standard, ERC721A is an improvement standard for ERC721 tokens. It was proposed by the Azuki team and used for developing their NFT collection. Compared with ERC721, ERC721A is a more gas-efficient standard to mint a lot of of NFTs simultaneously. It allows developers to mint multiple NFTs at the same gas price. This has been a great improvement due to Ethereum’s sky-rocketing gas fee.

Reference: https://nextrope.com/erc721-vs-erc721a-2/

[G-19] Optimize names to save gas

Contracts most called functions could simply save gas by function ordering via Method ID. Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions. 

Context:  All Contracts

Recommendation:  Find a lower method ID name for the most called functions for example Call() vs. Call1() is cheaper by 22 gas For example, the function IDs in the Namespace.sol contract will be the most used; A lower method ID may be given.

Proof of Consept: https://medium.com/joyso/solidity-how-does-function-name-affect-gas-consumption-in-smart-contract-47d270d8ac92

Namespace.sol function names can be named and sorted according to METHOD ID

Sighash   |   Function Signature
========================
c87b56dd  =>  tokenURI(uint256)
11f839e2  =>  fuse(CharacterData[])
42966c68  =>  burn(uint256)
21496de6  =>  changeNoteAddress(address)
92f8b9a3  =>  changeRevenueAddress(address)

[G-20] Upgrade Solidity's optimizer

Make sure Solidity’s optimizer is enabled. It reduces gas costs. If you want to gas optimize for contract deployment (costs less to deploy a contract) then set the Solidity optimizer at a low number. If you want to optimize for run-time gas costs (when functions are called on a contract) then set the optimizer to a high number.

3 results - 3 files:

canto-bio-protocol\foundry.toml:

  2: optimizer = true
  3: optimizer_runs = 1_000

canto-namespace-protocol\foundry.toml:

  2: optimizer = true
  3: optimizer_runs = 1_000

canto-pfp-protocol\foundry.toml:

  2: optimizer = true
  3: optimizer_runs = 1_000