Canto Identity Protocol contest - 0xackermann's results

Protocol Aggregating Protocol (PAP) for standardizing on-chain identity.

General Information

Platform: Code4rena

Start Date: 31/01/2023

Pot Size: $36,500 CANTO

Total HM: 5

Participants: 38

Period: 3 days

Judge: berndartmueller

Total Solo HM: 2

Id: 212

League: ETH

Canto Identity Protocol

Findings Distribution

Researcher Performance

Rank: 37/38

Findings: 1

Award: $27.36

Gas:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

Labels

bug
G (Gas Optimization)
grade-b
G-14

Awards

90.8172 CANTO - $27.36

External Links

Gas Optimizations Report

Excluded findings are below the automated findings.

Gas Optimizations

IssueInstances
GAS-1Use assembly to check for address(0)4
GAS-2Cache array length outside of loop2
GAS-3State variables should be cached in stack variables rather than re-reading them from storage1
GAS-4Use calldata instead of memory for function arguments that do not get mutated3
GAS-5Don't initialize variables with default value1
GAS-6Using private rather than public for constants, saves gas2
GAS-7Use != 0 instead of > 0 for unsigned integer comparison3

<a name="GAS-1"></a>[GAS-1] Use assembly to check for address(0)

Saves 6 gas per instance

Instances (4):

File: src/CidNFT.sol

137:         if (ownerOf[_id] == address(0))

176:         if (subprotocolOwner == address(0)) revert SubprotocolDoesNotExist(_subprotocolName);

248:         if (subprotocolOwner == address(0)) revert SubprotocolDoesNotExist(_subprotocolName);

Link to code

File: src/SubprotocolRegistry.sol

90:         if (subprotocolData.owner != address(0)) revert SubprotocolAlreadyExists(_name, subprotocolData.owner);

Link to code

<a name="GAS-2"></a>[GAS-2] Cache array length outside of loop

If not cached, the solidity compiler will always read the length of the array during each iteration. That is, if it is a storage array, this is an extra sload operation (100 additional extra gas for each iteration except for the first) and if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first).

Instances (2):

File: src/CidNFT.sol

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

214:             uint256 lengthBeforeAddition = activeData.values.length;

Link to code

<a name="GAS-3"></a>[GAS-3] State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second+ access of a state variable within a function. Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.

Saves 100 gas per instance

Instances (1):

File: src/CidNFT.sol

193:             SafeTransferLib.safeTransferFrom(note, msg.sender, subprotocolOwner, subprotocolFee - cidFee);

Link to code

<a name="GAS-4"></a>[GAS-4] Use calldata instead of memory for function arguments that do not get mutated

Mark data types as calldata instead of memory where possible. This makes it so that the data is not automatically loaded into memory. If the data passed into the function does not need to be changed (like updating values in an array), it can be passed in as calldata. The one exception to this is if the argument must later be passed into another function that takes an argument that specifies memory storage.

Instances (3):

File: src/CidNFT.sol

120:         string memory _name,

121:         string memory _symbol,

122:         string memory _baseURI,

Link to code

<a name="GAS-5"></a>[GAS-5] Don't initialize variables with default value

Instances (1):

File: src/CidNFT.sol

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

Link to code

<a name="GAS-6"></a>[GAS-6] Using private rather than public for constants, saves gas

If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table

Instances (2):

File: src/CidNFT.sol

17:     uint256 public constant CID_FEE_BPS = 1_000;

Link to code

File: src/SubprotocolRegistry.sol

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

Link to code

<a name="GAS-7"></a>[GAS-7] Use != 0 instead of > 0 for unsigned integer comparison

Instances (3):

File: src/AddressRegistry.sol

2: pragma solidity >=0.8.0;

Link to code

File: src/CidNFT.sol

2: pragma solidity >=0.8.0;

Link to code

File: src/SubprotocolRegistry.sol

2: pragma solidity >=0.8.0;

Link to code


Excluded Findings


[G-01] Usage of uint/int smaller than 32 bytes (256 bits) incurs overhead


Description:

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. 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.

Recommendation:

Use a larger size then downcast where needed

Lines of Code:


[G-02] ++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-loop and while-loops


Description:

In Solidity 0.8+, there’s a default overflow check on unsigned integers. It’s possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline.

Recommendation:

Using Solidity's unchecked block saves the overflow checks.

Proof Of Concept:

https://github.com/byterocket/c4-common-issues/blob/main/0-Gas-Optimizations.md#g011---unnecessary-checked-arithmetic-in-for-loop

Lines of Code:


[G-03] Optimize names to save gas [22 gas per instance]


Description:

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.

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

Proof Of Concept:

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

Lines of Code:


[G-04] Setting the constructor to payable [~13 gas per instance]


Lines of Code:


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


Description:

Solidity 0.8.10 has a useful change that reduced gas costs of external calls which expect 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.

Lines of Code:


[G-06] <array>.length should not be looked up in every loop of a for-loop


Description:

The overheads outlined below are PER LOOP, excluding the first loop

  • storage arrays incur a Gwarmaccess (100 gas)
  • memory arrays use MLOAD (3 gas)
  • calldata arrays use CALLDATALOAD (3 gas)

Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset

Lines of Code:

#0 - c4-judge

2023-02-18T12:42:16Z

berndartmueller marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter