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
Rank: 37/38
Findings: 1
Award: $27.36
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: HardlyDifficult
Also found by: 0xAgro, 0xSmartContract, 0xackermann, Aymen0909, Dravee, Matin, MiniGlome, NoamYakov, ReyAdmirado, Rolezn, Ruhum, c3phas, descharre, joestakey
90.8172 CANTO - $27.36
Excluded findings are below the automated findings.
Issue | Instances | |
---|---|---|
GAS-1 | Use assembly to check for address(0) | 4 |
GAS-2 | Cache array length outside of loop | 2 |
GAS-3 | State variables should be cached in stack variables rather than re-reading them from storage | 1 |
GAS-4 | Use calldata instead of memory for function arguments that do not get mutated | 3 |
GAS-5 | Don't initialize variables with default value | 1 |
GAS-6 | Using private rather than public for constants, saves gas | 2 |
GAS-7 | Use != 0 instead of > 0 for unsigned integer comparison | 3 |
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);
File: src/SubprotocolRegistry.sol 90: if (subprotocolData.owner != address(0)) revert SubprotocolAlreadyExists(_name, subprotocolData.owner);
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;
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);
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,
Instances (1):
File: src/CidNFT.sol 150: for (uint256 i = 0; i < _addList.length; ++i) {
private
rather than public
for constants, saves gasIf 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;
File: src/SubprotocolRegistry.sol 17: uint256 public constant REGISTER_FEE = 100 * 10**18;
Instances (3):
File: src/AddressRegistry.sol 2: pragma solidity >=0.8.0;
File: src/CidNFT.sol 2: pragma solidity >=0.8.0;
File: src/SubprotocolRegistry.sol 2: pragma solidity >=0.8.0;
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:
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:
Lines of Code:
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:
Lines of Code:
constructor
to payable
[~13 gas per instance]Lines of Code:
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:
<array>.length
should not be looked up in every loop of a for
-loopDescription:
The overheads outlined below are PER LOOP, excluding the first loop
MLOAD
(3 gas)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