code-423n4 / 2022-04-backed-findings

1 stars 1 forks source link

Gas Optimizations #119

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

1. Short require strings save gas

Impact

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.

Proof of Concept

Several cases of this gas optimization were found. These are a few examples, but more may exist

  1. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L84
  2. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L86
  3. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L121
  4. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L146-L148
  5. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L171-L172
  6. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L178
  7. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L189
  8. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L255
  9. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L259

Tools Used

Manual analysis

Recommended Mitigation Steps

Shorten require strings

2. Use != 0 instead of > 0

Impact

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

Proof of Concept

There were two instances of this issue https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L198 https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L321

Tools Used

grep

Recommended Mitigation Steps

Replace > 0 with != 0 to save gas

3. Add payable to functions that won't receive ETH

Impact

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.

Proof of Concept

There are many functions that have the onlyOwner modifier and can be payable setLendTicketContract https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L279 setBorrowTicketContract https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L289 withdrawOriginationFees https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L296 updateOriginationFeeRate https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L306 updateRequiredImprovementRate https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L320

Tools Used

Manual analysis

Recommended Mitigation Steps

Add payable to these functions for gas savings

4. Use Solidity errors instead of require

Impact

Solidity errors introduced in version 0.8.4 can save gas on revert conditions https://blog.soliditylang.org/2021/04/21/custom-errors/ https://twitter.com/PatrickAlphaC/status/1505197417884528640

Proof of Concept

Some error messages are used, but many require blocks still exist in the code. A few examples are

  1. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L84
  2. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L86
  3. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L121
  4. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L146-L148
  5. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L171-L172
  6. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L178
  7. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L189
  8. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L255
  9. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L259

Tools Used

Manual analysis

Recommended Mitigation Steps

Replace require blocks with new solidity errors described in https://blog.soliditylang.org/2021/04/21/custom-errors/

5. Public functions can be external

Impact

Declaring a function as external instead of public saves gas

Proof of Concept

One function can be declared external instead of public

Tools Used

Manual analysis

Recommended Mitigation Steps

Change function from public to external to save gas

gzeoneth commented 2 years ago

3,4,5 is valid

wilsoncusack commented 2 years ago

3 - yes but these are infrequently called so do not care about optimizing + I think labeling functions that are not intended to take eth as payable is an anti pattern 4 - I shorted revert messages to free up byte space for the optimizer but in general do not care about optimizing revert gas 5 - function needs to be marked public in order to override the inherited