Canto Identity Protocol contest - Matin's results

Summary

Low-Risk Issues

Total: 7 instances over 1 issue

Non-critical Issues

Total: 8 instances over 4 issues

Note: The table above was created considering the automatic findings and thus, those are not included.

Low-Risk Issues

[L‑01] indexed keyword for reference type variables returns the obscure 32 bytes hash of the string

when the indexed keyword is used for reference typed variables such as string, it will return the hash of the mentioned string. Thus, the event which is supposed to inform all of the applications subscribed to its emitting transaction (e.g. front-end of the DApp), would get a meaningless and obscure 32 bytes that correspond to keccak256 of an encoded string. For more information about the indexed events, one can check here(https://docs.soliditylang.org/en/v0.8.17/abi-spec.html?highlight=indexed#events).

There are 7 instances of this issue:

    event SubprotocolRegistered(
        address indexed registrar,
        string indexed name,
        address indexed nftAddress,
        bool ordered,
        bool primary,
        bool active,
        uint96 fee
    );

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/SubprotocolRegistry.sol#L47

    event OrderedDataAdded(
        uint256 indexed cidNFTID,
        string indexed subprotocolName,
        uint256 indexed key,
        uint256 subprotocolNFTID
    );

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L77

    event PrimaryDataAdded(uint256 indexed cidNFTID, string indexed subprotocolName, uint256 subprotocolNFTID);

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L81

    event ActiveDataAdded(
        uint256 indexed cidNFTID,
        string indexed subprotocolName,
        uint256 subprotocolNFTID,
        uint256 arrayIndex
    );

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L84

    event OrderedDataRemoved(
        uint256 indexed cidNFTID,
        string indexed subprotocolName,
        uint256 indexed key,
        uint256 subprotocolNFTID
    );

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L90

    event PrimaryDataRemoved(uint256 indexed cidNFTID, string indexed subprotocolName, uint256 subprotocolNFTID);

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L94

    event ActiveDataRemoved(uint256 indexed cidNFTID, string indexed subprotocolName, uint256 subprotocolNFTID);

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L95

Non-critical Issues

[N‑01] Use a more recent version of solidity.

Using version 0.8.17 for the solidity compiler is better.

There are 4 instances of this issue:

    pragma solidity >=0.8.0;

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L2 https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/SubprotocolRegistry.sol#L2 https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/AddressRegistry.sol#L2 https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidSubprotocolNFT.sol#L2 https://github.com/with-backed/papr/blob/1933da2e38ff9d47c17e2749d6088bbbd40bfa68/src/libraries/OracleLibrary.sol#L22

[N‑02] Use scientific notation (e.g. 1e18) rather than exponential form (e.g. 10**18)

There is 1 instance of this issue:

    uint256 public constant REGISTER_FEE = 100 * 10**18;

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/SubprotocolRegistry.sol#L17

[N‑03] File is missing NatSpec

Some functions miss NatSpec (@inheritdoc)

There are 2 instances of this issue:

    function onERC721Received(
        address, /*operator*/
        address, /*from*/
        uint256, /*id*/
        bytes calldata /*data*/
    ) external pure returns (bytes4) {
        return ERC721TokenReceiver.onERC721Received.selector;
    }

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L339

    function supportsInterface(bytes4 interfaceId) public pure override returns (bool) {
        return interfaceId == CID_SUBPROTOCOL_INTERFACE_ID || super.supportsInterface(interfaceId);
    }

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidSubprotocolNFT.sol#L16

[N‑04] Consider using delete rather than assigning zero to default values

There is 1 instance of this issue:

    activeData.positions[_nftIDToRemove] = 0;

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L284

Summary

Gas Optimizations

Total: 15 instances over 5 issues with 2520 gas saved

The table above as well as its gas numbers are created by considering the automatic findings which are not included.

Gas Optimizations

[G‑01] Storing the struct value of the mapping as storage

Storing the struct value of the mapping as storage costs less than memory inside a function. Declaring the subprotocols[_name] as memory and re-assigning the values to it at the end of the function costs about 2000 gas more than the case in which it is defined as storage (storage pointer for the struct value part of mapping).

There is 1 instance of this issue:

    SubprotocolData memory subprotocolData = subprotocols[_name];

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/SubprotocolRegistry.sol#L89

[G‑02] Using uint256 costs low gas rather than enum types (or uint8)

Using uint256 costs low gas rather than enum types (or uint8) for condition checks inside contract CidNFT. As the compiler bypasses the explicit uint8 to `uint256``` conversion inside the conditional terms, it saves some gas.

There are 6 instances of this issue:

    if (_type == AssociationType.ORDERED)

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L195

    else if (_type == AssociationType.PRIMARY)

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L203

    else if (_type == AssociationType.ACTIVE)

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L211

    if (_type == AssociationType.ORDERED)

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L257

   else if (_type == AssociationType.PRIMARY)

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L266

    else if (_type == AssociationType.ACTIVE)

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L272

[G‑03] Keyword delete for mappings[_key] may save some tiny amount of gas rather than make it zero manually

By using the keyword delete for a specific key of a mapping, the value of that mapping would be re-initialized to its default value.

There is 1 instance of this issue:

    activeData.positions[_nftIDToRemove] = 0;

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L284

[G‑04] Setting the constructor to be payable

Save some gas by setting the constructor to be payable.

There are 3 instances of this issue:

    constructor(
        string memory _name,
        string memory _symbol,
        string memory _baseURI,
        address _cidFeeWallet,
        address _noteContract,
        address _subprotocolRegistry
    ) ERC721(_name, _symbol) {

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L119

    constructor(address _noteContract, address _cidFeeWallet) {
        note = ERC20(_noteContract);
        cidFeeWallet = _cidFeeWallet;
    }

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/SubprotocolRegistry.sol#L65

    constructor(address _cidNFT) {
        cidNFT = _cidNFT;
    }

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/AddressRegistry.sol#L36

[G‑05] Use solidity version 0.8.17 to save some gas

Set the solidity compiler version to the recent version 0.8.17.

There are 4 instances of this issue:

    pragma solidity >=0.8.0;

https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidNFT.sol#L2 https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/SubprotocolRegistry.sol#L2 https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/AddressRegistry.sol#L2 https://github.com/code-423n4/2023-01-canto-identity/blob/main/src/CidSubprotocolNFT.sol#L2