Cally contest - TerrierLover's results

Here are 9 findings regarding potential gas optimizations. The report is written per the contract file.

---

Cally.sol

[Gas-1] Setting 0 on uint variable is not needed

Following codes set 0 on uint256 variables. The default value of uint256 variable is 0, so removing them can reduce the deployment gas cost.

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L94-L95

uint256 public feeRate = 0;
uint256 public protocolUnclaimedFees = 0;

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L282

uint256 fee = 0;

---

[Gas-2] Use !=0 instead of > 0

Following codes use > 0 on uint256 variables. uint256 variables will not be less than 0, so simply using !=0 can reduce the deployment gas cost.

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L170

require(durationDays > 0, "durationDays too small");

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L283

if (feeRate > 0) {

---

[Gas-3] Use custom errors

Using custom errors can reduce the gas cost.

There are 28 callsites of require check with inline error message. Using the custom errors instead of the inline error messages can reduce the gas cost.

---

[Gas-4] Use ! instead of == false

Following codebase use == false to check if the value is false. Using ! instead of of == false can reduce the gas cost.

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L217

require(vault.isExercised == false, "Vault already exercised");

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L220

require(vault.isWithdrawing == false, "Vault is being withdrawn");

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L328

require(vault.isExercised == false, "Vault already exercised");

The proposed workaround is like this:

require(!vault.isExercised, "Vault already exercised");

---

[Gas-5] Access _valuts state variable after the input arguments check

Vault memory vault = _vaults[vaultId] comes before the require checks.

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L208

function buyOption(uint256 vaultId) external payable returns (uint256 optionId) {
    Vault memory vault = _vaults[vaultId];

    // vaultId should always be odd
    require(vaultId % 2 != 0, "Not vault type");

_vaults[vaultId] can be accessed after the first two require checks. By doing so, it can reduce the gas cost of the transaction that fails for the first two checks.

function buyOption(uint256 vaultId) external payable returns (uint256 optionId) {
    // vaultId should always be odd
    require(vaultId % 2 != 0, "Not vault type");

    // check vault exists
    require(ownerOf(vaultId) != address(0), "Vault does not exist");

    Vault memory vault = _vaults[vaultId];

    // check that the vault still has the NFTs as collateral
    require(vault.isExercised == false, "Vault already exercised");

    // check that the vault is not in withdrawing state
    require(vault.isWithdrawing == false, "Vault is being withdrawn");

---

[Gas-6] Defining variables are not needed

Following codebase defines variables (premium, auctionStartTimestamp and optionId) but they are only used 1 time.

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L223-L224

uint256 premium = getPremium(vaultId);
require(msg.value >= premium, "Incorrect ETH amount sent");

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L227-L228

uint32 auctionStartTimestamp = vault.currentExpiration;
require(block.timestamp >= auctionStartTimestamp, "Auction not started");

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L333-L334

uint256 optionId = vaultId + 1;
_burn(optionId);

Not defining variables like followings can reduce the gas cost.

require(msg.value >= getPremium(vaultId), "Incorrect ETH amount sent");

require(block.timestamp >= vault.currentExpiration, "Auction not started");

_burn(vaultId + 1);

---

CallyNFT.sol

[Gas-7] Setting 0 on uint variables is not needed

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/CallyNft.sol#L244

for (uint256 i = 0; i < data.length; i++) {

It sets uint256 i = 0 at the for loop, but setting 0 is not needed. Rewriting as follows can reduce the gas cost.

for (uint256 i; i < data.length; i++) {

---

[Gas-8] Use fixed value instead of calculating the length of address

2 + data.length * 2 is used to calculate the length of address.

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/CallyNft.sol#L241

bytes memory str = new bytes(2 + data.length * 2);

address type has a fixed length, so rewriting as follows can reduce the gas cost.

bytes memory str = new bytes(42);

---

[Gas-9] Consider using private or internal for addressToString and renderSvg functions

addressToString and renderSvg functions seem to be needed only inside this contract.

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/CallyNft.sol#L236

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/CallyNft.sol#L136-L146

If this is true, by changing the visibility to internal can reduce the gas cost.

---