Canto Identity Protocol contest - Rolezn's results

Summary<a name="Summary">

Low Risk Issues

Total: 18 contexts over 56 issues

Non-critical Issues

Total: 19 contexts over 10 issues

Low Risk Issues

<a href="#Summary">[LOW‑1]</a><a name="LOW&#x2011;1"> Add to blacklist function

NFT thefts have increased recently, so with the addition of hacked NFTs to the platform, NFTs can be converted into liquidity. To prevent this, I recommend adding the blacklist function.

Marketplaces such as Opensea have a blacklist feature that will not list NFTs that have been reported theft, NFT projects such as Manifold have blacklist functions in their smart contracts.

Here is the project example; Manifold

Manifold Contract https://etherscan.io/address/0xe4e4003afe3765aca8149a82fc064c0b125b9e5a#code

     modifier nonBlacklistRequired(address extension) {
         require(!_blacklistedExtensions.contains(extension), "Extension blacklisted");
         _;
     }

<ins>Recommended Mitigation Steps</ins>

Add to Blacklist function and modifier.

<a href="#Summary">[LOW‑2]</a><a name="LOW&#x2011;2"> Possible rounding issue

There might be a rounding issue as CID_FEE_BPS = 1000, however there is no defined value for subprotocolFee and it could be 0 or uint96.max. But values between 1-9 for subprotocolFee will result in cidFee being 0 due to rounding issue.

<ins>Proof Of Concept</ins>

191: uint256 cidFee = (subprotocolFee * CID_FEE_BPS) / 10_000;

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

<a href="#Summary">[LOW‑3]</a><a name="LOW&#x2011;3"> Missing parameter validation in constructor

Some parameters of constructors are not checked for invalid values.

<ins>Proof Of Concept</ins>

36: address _cidNFT

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

123: address _cidFeeWallet
124: address _noteContract
125: address _subprotocolRegistry

https://github.com/code-423n4/2023-01-canto-identity/tree/main/src/CidNFT.sol#L123-L125

65: address _noteContract
65: address _cidFeeWallet

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

<ins>Recommended Mitigation Steps</ins>

Validate the parameters.

<a href="#Summary">[LOW‑4]</a><a name="LOW&#x2011;4"> Protect your NFT from copying in POW forks

Ethereum has performed the long-awaited "merge" that will dramatically reduce the environmental impact of the network

There may be forked versions of Ethereum, which could cause confusion and lead to scams as duplicated NFT assets enter the market.

If the Ethereum Merge, which took place in September 2022, results in the Blockchain splitting into two Blockchains due to the 'THE DAO' attack in 2016, this could result in duplication of immutable tokens (NFTs).

In any case, duplicate NFTs will exist due to the ETH proof-of-work chain and other potential forks, and there’s likely to be some level of confusion around which assets are 'official' or 'authentic.'

Even so, there could be a frenzy for these copies, as NFT owners attempt to flip the proof-of-work versions of their valuable tokens.

As ETHPOW and any other forks spin off of the Ethereum mainnet, they will yield duplicate versions of Ethereum’s NFTs. An NFT is simply a blockchain token, and it can work as a deed of ownership to digital items like artwork and collectibles. A forked Ethereum chain will thus have duplicated deeds that point to the same tokenURI

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

<ins>Proof Of Concept</ins>

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

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

<ins>Recommended Mitigation Steps</ins>

Add the following check:

if(block.chainid != 1) { 
    revert(); 
}

<a href="#Summary">[LOW‑5]</a><a name="LOW&#x2011;5"> 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

<ins>Proof Of Concept</ins>

6: import "solmate/utils/SafeTransferLib.sol";

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

6: import "solmate/utils/SafeTransferLib.sol";

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

<ins>Recommended Mitigation Steps</ins>

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;
}

Non Critical Issues

<a href="#Summary">[NC‑1]</a><a name="NC&#x2011;1"> Function writing that does not comply with the Solidity Style Guide

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

<ins>Proof Of Concept</ins>

All in-scope contracts

<a href="#Summary">[NC‑2]</a><a name="NC&#x2011;2"> Use delete to Clear Variables

delete a assigns the initial value for the type to a. i.e. for integers it is equivalent to a = 0, but it can also be used on arrays, where it assigns a dynamic array of length zero or a static array of the same length with all elements reset. For structs, it assigns a struct with all members reset. Similarly, it can also be used to set an address to zero address. It has no effect on whole mappings though (as the keys of mappings may be arbitrary and are generally unknown). However, individual keys and what they map to can be deleted: If a is a mapping, then delete a[x] will delete the value stored at x.

The delete key better conveys the intention and is also more idiomatic. Consider replacing assignments of zero with delete statements.

<ins>Proof Of Concept</ins>

284: activeData.positions[_nftIDToRemove] = 0;

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

<a href="#Summary">[NC‑3]</a><a name="NC&#x2011;3"> NatSpec return parameters should be included in contracts

It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability.

https://docs.soliditylang.org/en/v0.8.15/natspec-format.html

<ins>Proof Of Concept</ins>

All in-scope contracts

<ins>Recommended Mitigation Steps</ins>

Include return parameters in NatSpec comments

Recommendation Code Style: (from Uniswap3)

    /// @notice Adds liquidity for the given recipient/tickLower/tickUpper position
    /// @dev The caller of this method receives a callback in the form of IUniswapV3MintCallback#uniswapV3MintCallback
    /// in which they must pay any token0 or token1 owed for the liquidity. The amount of token0/token1 due depends
    /// on tickLower, tickUpper, the amount of liquidity, and the current price.
    /// @param recipient The address for which the liquidity will be created
    /// @param tickLower The lower tick of the position in which to add liquidity
    /// @param tickUpper The upper tick of the position in which to add liquidity
    /// @param amount The amount of liquidity to mint
    /// @param data Any data that should be passed through to the callback
    /// @return amount0 The amount of token0 that was paid to mint the given amount of liquidity. Matches the value in the callback
    /// @return amount1 The amount of token1 that was paid to mint the given amount of liquidity. Matches the value in the callback
    function mint(
        address recipient,
        int24 tickLower,
        int24 tickUpper,
        uint128 amount,
        bytes calldata data
    ) external returns (uint256 amount0, uint256 amount1);

i.e.

    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/tree/main/src/CidNFT.sol#L339

<a href="#Summary">[NC‑4]</a><a name="NC&#x2011;4"> Lines are too long

Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length Reference: https://docs.soliditylang.org/en/v0.8.10/style-guide.html#maximum-line-length

<ins>Proof Of Concept</ins>

185: // The CID Protocol safeguards the NFTs of subprotocols. Note that these NFTs are usually pointers to other data / NFTs (e.g., to an image NFT for profile pictures)

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

78: /// @param _fee Fee (in $NOTE) for minting a new token of the subprotocol. Set to 0 if there is no fee. 10% is subtracted from this fee as a CID fee

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

<a href="#Summary">[NC‑5]</a><a name="NC&#x2011;5"> Implementation contract may not be initialized

OpenZeppelin recommends that the initializer modifier be applied to constructors. Per OZs Post implementation contract should be initialized to avoid potential griefs or exploits. https://forum.openzeppelin.com/t/uupsupgradeable-vulnerability-post-mortem/15680/5

<ins>Proof Of Concept</ins>

36: constructor(address _cidNFT)

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

119: 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/tree/main/src/CidNFT.sol#L119

65: constructor(address _noteContract, address _cidFeeWallet)

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

<a href="#Summary">[NC‑6]</a><a name="NC&#x2011;6"> NatSpec comments should be increased in contracts

It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability. https://docs.soliditylang.org/en/v0.8.15/natspec-format.html

<ins>Proof Of Concept</ins>

All in-scope contracts

i.e. Natspec documentation missing info which includes about @param _cidNFTID

    /// @notice Register a CID NFT to the address of the caller. NFT has to be owned by the caller
    /// @dev Will overwrite existing registration if any exists
    function register(uint256 _cidNFTID) external {
        if (ERC721(cidNFT).ownerOf(_cidNFTID) != msg.sender)
            // We only guarantee that a CID NFT is owned by the user at the time of registration
            // ownerOf reverts if non-existing ID is provided
            revert NFTNotOwnedByUser(_cidNFTID, msg.sender);
        cidNFTs[msg.sender] = _cidNFTID;
        emit CIDNFTAdded(msg.sender, _cidNFTID);
    }

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

    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/tree/main/src/CidNFT.sol#L339

<ins>Recommended Mitigation Steps</ins>

NatSpec comments should be increased in contracts

<a href="#Summary">[NC‑7]</a><a name="NC&#x2011;7"> Non-usage of specific imports

The current form of relative path import is not recommended for use because it can unpredictably pollute the namespace. Instead, the Solidity docs recommend specifying imported symbols explicitly. https://docs.soliditylang.org/en/v0.8.15/layout-of-source-files.html#importing-other-source-files

A good example:

import {OwnableUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol";
import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol";
import {SafeCastLib} from "solmate/utils/SafeCastLib.sol";
import {ERC20} from "solmate/tokens/ERC20.sol";
import {IProducer} from "src/interfaces/IProducer.sol";
import {GlobalState, UserState} from "src/Common.sol";

<ins>Proof Of Concept</ins>

7: import "./SubprotocolRegistry.sol";

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

7: import "./CidSubprotocolNFT.sol";

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

<ins>Recommended Mitigation Steps</ins>

Use specific imports syntax per solidity docs recommendation.

<a href="#Summary">[NC‑8]</a><a name="NC&#x2011;8"> Use a more recent version of Solidity

<a href="https://blog.soliditylang.org/2021/04/21/solidity-0.8.4-release-announcement/">0.8.4</a>: bytes.concat() instead of abi.encodePacked(<bytes>,<bytes>)

<a href="https://blog.soliditylang.org/2022/02/16/solidity-0.8.12-release-announcement/">0.8.12</a>: string.concat() instead of abi.encodePacked(<str>,<str>)

<a href="https://blog.soliditylang.org/2022/03/16/solidity-0.8.13-release-announcement/">0.8.13</a>: Ability to use using for with a list of free functions

<a href="https://blog.soliditylang.org/2022/05/18/solidity-0.8.14-release-announcement/">0.8.14</a>:

ABI Encoder: When ABI-encoding values from calldata that contain nested arrays, correctly validate the nested array length against calldatasize() in all cases. Override Checker: Allow changing data location for parameters only when overriding external functions.

<a href="https://blog.soliditylang.org/2022/06/15/solidity-0.8.15-release-announcement/">0.8.15</a>:

Code Generation: Avoid writing dirty bytes to storage when copying bytes arrays. Yul Optimizer: Keep all memory side-effects of inline assembly blocks.

<a href="https://blog.soliditylang.org/2022/08/08/solidity-0.8.16-release-announcement/">0.8.16</a>:

Code Generation: Fix data corruption that affected ABI-encoding of calldata values represented by tuples: structs at any nesting level; argument lists of external functions, events and errors; return value lists of external functions. The 32 leading bytes of the first dynamically-encoded value in the tuple would get zeroed when the last component contained a statically-encoded array.

<a href="https://blog.soliditylang.org/2022/09/08/solidity-0.8.17-release-announcement/">0.8.17</a>:

Yul Optimizer: Prevent the incorrect removal of storage writes before calls to Yul functions that conditionally terminate the external EVM call.

<ins>Proof Of Concept</ins>

pragma solidity >=0.8.0;

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

pragma solidity >=0.8.0;

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

pragma solidity >=0.8.0;

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

<ins>Recommended Mitigation Steps</ins>

Consider updating to a more recent solidity version.

<a href="#Summary">[NC‑9]</a><a name="NC&#x2011;9"> Use bytes.concat()

Solidity version 0.8.4 introduces bytes.concat() (vs abi.encodePacked(<bytes>,<bytes>))

<ins>Proof Of Concept</ins>

140: return string(abi.encodePacked(baseURI, _id, ".json")

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

<ins>Recommended Mitigation Steps</ins>

Use bytes.concat() and upgrade to at least Solidity version 0.8.4 if required.

<a href="#Summary">[NC‑10]</a><a name="NC&#x2011;10"> Use standard decimals interpretations using 1eX

Use 1eX instead of 1**X

<ins>Proof Of Concept</ins>

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

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

<ins>Recommended Mitigation Steps</ins>

Replace 10**X with 1eX

Summary<a name="Summary">

Gas Optimizations

Total: 24 contexts over 11 issues

Gas Optimizations

<a href="#Summary">[GAS‑1]</a><a name="GAS&#x2011;1"> State variables only set in the constructor should be declared immutable

Avoids a Gsset (20000 gas) in the constructor, and replaces each Gwarmacces (100 gas) with a PUSH32 (3 gas).

<ins>Proof Of Concept</ins>

127: baseURI = _baseURI

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

<a href="#Summary">[GAS‑2]</a><a name="GAS&#x2011;2"> Setting the constructor to payable

Saves ~13 gas per instance

<ins>Proof Of Concept</ins>

36: constructor(address _cidNFT)

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

119: 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/tree/main/src/CidNFT.sol#L119

65: constructor(address _noteContract, address _cidFeeWallet)

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

<a href="#Summary">[GAS‑3]</a><a name="GAS&#x2011;3"> Do not calculate constants

Due to how constant variables are implemented (replacements at compile-time), an expression assigned to a constant variable is recomputed each time that the variable is used, which wastes some gas.

<ins>Proof Of Concept</ins>

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

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

<a href="#Summary">[GAS‑4]</a><a name="GAS&#x2011;4"> Using delete statement can save gas

<ins>Proof Of Concept</ins>

284: activeData.positions[_nftIDToRemove] = 0;

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

<a href="#Summary">[GAS‑5]</a><a name="GAS&#x2011;5"> Use hardcode address instead address(this)

Instead of using address(this), it is more gas-efficient to pre-calculate and use the hardcoded address. Foundry's script.sol and solmate's LibRlp.sol contracts can help achieve this.

References: https://book.getfoundry.sh/reference/forge-std/compute-create-address

https://twitter.com/transmissions11/status/1518507047943245824

<ins>Proof Of Concept</ins>

154: ) = address(this).delegatecall(abi.encodePacked(addSelector, _addList[i]));

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

187: nftToAdd.safeTransferFrom(msg.sender, address(this), _nftIDToAdd);

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

264: nftToRemove.safeTransferFrom(address(this), msg.sender, currNFTID);
264: nftToRemove.safeTransferFrom(address(this), msg.sender, currNFTID);
285: nftToRemove.safeTransferFrom(address(this), msg.sender, _nftIDToRemove);

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

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

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

<ins>Recommended Mitigation Steps</ins>

Use hardcoded address

<a href="#Summary">[GAS‑6]</a><a name="GAS&#x2011;6"> 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.

See more <a href="https://medium.com/joyso/solidity-how-does-function-name-affect-gas-consumption-in-smart-contract-47d270d8ac92">here</a>

<ins>Proof Of Concept</ins>

All in-scope contracts

<ins>Recommended Mitigation Steps</ins>

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 Gauge.sol contract will be the most used; A lower method ID may be given.

<a href="#Summary">[GAS‑7]</a><a name="GAS&#x2011;7"> Public Functions To External

The following functions could be set external to save gas and improve code quality. External call cost is less expensive than of public functions.

<ins>Proof Of Concept</ins>

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

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

function remove(
        uint256 _cidNFTID,
        string calldata _subprotocolName,
        uint256 _key,
        uint256 _nftIDToRemove,
        AssociationType _type
    ) public {

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

<a href="#Summary">[GAS‑8]</a><a name="GAS&#x2011;8"> Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract's 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 Each operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed

<ins>Proof Of Concept</ins>

189: uint96 subprotocolFee = subprotocolData.fee;

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

32: uint96 fee;

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

<a href="#Summary">[GAS‑9]</a><a name="GAS&#x2011;9"> ++i/i++ Should Be unchecked{++i}/unchecked{i++} When It Is Not Possible For Them To Overflow, As Is The Case When Used In For- And While-loops

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas PER LOOP

<ins>Proof Of Concept</ins>

150: for (uint256 i = 0; i < _addList.length; ++i) {

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

<a href="#Summary">[GAS‑10]</a><a name="GAS&#x2011;10"> Use solidity version 0.8.17 to gain some gas boost

Upgrade to the latest solidity version 0.8.17 to get additional gas savings.

<ins>Proof Of Concept</ins>

pragma solidity >=0.8.0;

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

pragma solidity >=0.8.0;

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

pragma solidity >=0.8.0;

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

<a href="#Summary">[GAS‑11]</a><a name="GAS&#x2011;11"> Using storage instead of memory saves gas

When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read. Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct

<ins>Proof Of Concept</ins>

89: SubprotocolData memory subprotocolData = subprotocols[_name];

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

<a href="#Summary">[GAS‑12]</a><a name="GAS&#x2011;12"> Using 10**X for constants isn't gas efficient

In Solidity, a constant expression in a variable will compute the expression everytime the variable is called. It's not the result of the expression that is stored, but the expression itself.

As Solidity supports the scientific notation, constants of form 10**X can be rewritten as 1eX to save the gas cost from the calculation with the exponentiation operator **.

<ins>Proof Of Concept</ins>

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

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

<ins>Recommended Mitigation Steps</ins>

Replace 10**X with 1eX