Cally contest - 0xkatana's results

[G-01] Redundant zero initialization

Solidity does not recognize null as a value, so uint variables are initialized to zero. Setting a uint variable to zero is redundant and can waste gas.

There was one place where an int is initialized to zero https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/Cally.sol#L94 https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/Cally.sol#L95 https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/Cally.sol#L282 https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/CallyNft.sol#L244

Recommended Mitigation Steps

Remove the redundant zero initialization uint256 i; instead of uint256 i = 0;

[G-02] Short require strings save gas

Strings in solidity are handled in 32 byte chunks. A require string longer than 32 bytes uses more gas. Shortening these strings will save gas.

One instance was found https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/CallyNft.sol#L89

Recommended Mitigation Steps

Shorten all require strings to less than 32 characters

[G-03] Use prefix not postfix in loops

Using a prefix increment (++i) instead of a postfix increment (i++) saves gas for each loop cycle and so can have a big gas impact when the loop executes on a large number of elements.

There is one example of this in for loops https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/Cally.sol#L244

Recommended Mitigation Steps

Use prefix not postfix to increment in a loop

[G-04] Use != 0 instead of > 0

Using > 0 uses slightly more gas than using != 0. Use != 0 when comparing uint variables to zero, which cannot hold values below zero

Locations where this was found include https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/Cally.sol#L170 https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/Cally.sol#L283

Recommended Mitigation Steps

Replace > 0 with != 0 to save gas

[G-05] Add payable to functions that won't receive ETH

Identifying a function as payable saves gas. Functions that have the onlyOwner modifier cannot be called by normal users and will not mistakenly receive ETH. These functions can be payable to save gas. This is especially relevant because withdrawAll functions exist to withdraw any ETH accidentally sent.

There are many functions that have the onlyOwner modifier in the contracts. Some examples are https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/Cally.sol#L119 https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/Cally.sol#L124

Recommended Mitigation Steps

Add payable to these functions for gas savings

[G-06] Cache array length before loop

Caching the array length outside a loop saves reading it on each iteration, as long as the array's length is not changed during the loop. This saves gas.

This was found in one place https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/Cally.sol#L244

Recommended Mitigation Steps

Cache the array length before the for loop

[G-07] Use existing cached vault value

getPremium caches the value of _vaults[vaultId] and then returns the value premiumOptions[vault.premiumIndex]. But everywhere that getPremium is used, the _vaults[vaultId] value is already cached, so using the value premiumOptions[vault.premiumIndex] directly saves gas by removing a function call.

There are two places where this improvement can be made https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/Cally.sol#L223 https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/Cally.sol#L464

Recommended Mitigation Steps

Replace getPremium calls with the value premiumOptions[vault.premiumIndex]