Gas Optimizations

[code423n4]

1. Loops can be more efficient

Impact

The local variable used as for loop index need not be initialized to 0 because the default value is 0. Avoiding this anti-pattern can save a few opcodes and therefore a tiny bit of gas.

Proof of Concept

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

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/LensHub.sol#L541

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/modules/follow/ApprovalFollowModule.sol#L41

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/modules/follow/ApprovalFollowModule.sol#L66

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/modules/follow/ApprovalFollowModule.sol#L128

The same situation are in other scope contracts where loops use.
Remix

Recommended Mitigation Steps

Remove explicit 0 initialization of for loop index variable.

2. Long Revert Strings

Impact

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition has been met.

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/base/ERC721Time.sol#L77

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/base/ERC721Time.sol#L86

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/base/ERC721Time.sol#L95

There are several other places throughout the codebase where the same optimization can be used.

Tools

https://planetcalc.com/9029/

Recommended Mitigation Steps

Shorten the revert strings to fit in 32 bytes.

3. > 0 can be replaced with != 0 for gas optimisation

Vulnerability details

Impact

!= 0 is a cheaper operation compared to > 0, when dealing with uint.

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/modules/follow/ApprovalFollowModule.sol#L64
https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/base/ERC721Time.sol#L135

Tools

Remix

Recommended Mitigation Steps

4. Adding unchecked directive can save gas

Impact

For the arithmetic operations that will never over/underflow, using the unchecked directive (Solidity v0.8 has default overflow/underflow checks) can save some gas from the unnecessary internal over/underflow checks.

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/modules/collect/LimitedTimedFeeCollectModule.sol#L72

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/modules/collect/TimedFeeCollectModule.sol#L71

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/FollowNFT.sol#L106-L145

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/FollowNFT.sol#L148-L187

Tools

Remix

Recommended Mitigation Steps

Consider using 'unchecked' where it is safe to do so.

5. A variable is being assigned its default value

Vulnerability details

Impact

A variable is being assigned its default value which is unnecessary.
Removing the assignment will save gas when deploying.

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/upgradeability/VersionedInitializable.sol#L29

uint256 private lastInitializedRevision = 0;

Tools

Remix

Recommended Mitigation Steps

Remove the assignment.

6. Less than 256 uints are not gas efficient

Vulnerability details

Impact

Lower than uint256 size storage instance variables are actually less gas efficient. E.g. using uint24 does not give any efficiency, actually, it is the opposite as EVM operates on default of 256-bit values so uint24 is more expensive in this case as it needs a conversion. It only gives improvements in cases where you can pack variables together, e.g. structs.

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/modules/collect/LimitedTimedFeeCollectModule.sol#L48
https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/libraries/Constants.sol#L10
https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/modules/FeeModuleBase.sol#L17

uint24 internal constant ONE_DAY = 24 hours;

Tools

Remix

Recommended Mitigation Steps

Consider to review all uint types. Change them with uint256 If the integer is not necessary to present with uint24.

7. Change string to byteX if possible

Vulnerability details

Impact

In the Constants library, declaring the constants with type bytes32 can save gas.

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/libraries/Constants.sol#L6-L9

Tools

https://medium.com/layerx/how-to-reduce-gas-cost-in-solidity-f2e5321e0395#2a78

Recommended Mitigation Steps

8. Modifier whenNotPaused and function _validateNotPaused

Vulnerability details

Impact

modifier whenNotPaused calls internal function _validateNotPaused. This function is not called anywhere else so I do not see a reason why all the logic can't be moved to the modifier to save some gas by reducing the extra call.

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/base/LensMultiState.sol#L18
The same in:
https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/base/LensMultiState.sol#L23

modifier whenNotPaused() {
        _validateNotPaused();
        _;
    }

Tools

Remix

Recommended Mitigation Steps

Remove function _validateNotPaused, place its logic directly in the modifier whenNotPaused`.

9. Checking non-zero value can avoid an external call to save gas

Vulnerability details

Impact

Checking if adjustedAmount and treasuryAmount > 0 before making the external library call to IERC20(currency) can save gas by avoiding the external call in such situations.

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/modules/collect/FeeCollectModule.sol#L135-L136
https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/modules/collect/LimitedTimedFeeCollectModule.sol#L164-L165

There are several other places throughout the codebase where the same optimization can be used.

Tools

Remix

Recommended Mitigation Steps

10. Change encode to encodePacked

Vulnerability details

Impact

Change abi.encode to abi.encodePacked can save gas.

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/FollowNFT.sol#L86
https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/LensHub.sol#L185
There are several other places throughout the codebase where the same optimization can be used.

Tools

Remix

Recommended Mitigation Steps

11. Use calldata instead of memory for external functions where the function argument is read-only

Vulnerability details

Impact

On external functions, when using the memory keyword with a function argument, what's happening is that a memory acts as an intermediate.
Reading directly from calldata using calldataload instead of going via memory saves the gas from the intermediate memory operations that carry the values.
As an extract from https://ethereum.stackexchange.com/questions/74442/when-should-i-use-calldata-and-when-should-i-use-memory :
«memory and calldata (as well as storage) are keywords that define the data area where a variable is stored. To answer your question directly, memory should be used when declaring variables (both function parameters as well as inside the logic of a function) that you want stored in memory (temporary), and calldata must be used when declaring an external function's dynamic parameters. The easiest way to think about the difference is that calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory.»

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/libraries/PublishingLogic.sol#L124

Tools

Remix

Recommended Mitigation Steps

Use calldata instead of memory for external functions where the function argument is read-only.

12. Changing bool to uint256 can save gas

Vulnerability details

Impact

Booleans are more expensive than uint256 or any type that takes up a full word because each write operation emits an extra SLOAD to first read the slot's contents, replace the bits taken up by the boolean, and then write back. This is the compiler's defense against contract upgrades and pointer aliasing, and it cannot be disabled.

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/storage/LensHubStorage.sol#L59-L62

Tools

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/ReentrancyGuard.sol#L23-L27

Recommended Mitigation Steps

13. Internal functions can be private if the contract is not herited

Vulnerability details

Impact

Several internal functions are in contracts that are never inherited. Private functions are cheaper than internal functions. Therefore, their visibility should be reduced to private.

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/libraries/PublishingLogic.sol#L380
https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/FollowNFT.sol#L225

Tools

Remix

Recommended Mitigation Steps

Define these functions as private.

14. Caching variables

Impact

Some of the variable can be cached to slightly reduce gas usage.

Proof of Concept

HUB can be cached.
https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/FollowNFT.sol#L207-L212

Tools

Remix

Recommended Mitigation Steps

Consider caching those variable for read and make sure write back to storage.

[Zer0dot]

The point about unchecked is valid!

A lot of the rest is kind of valid, but for some clarifications:

1. We use memory in some external library functions specifically to avoid stack too deep errors.
2. We cannot change encoding in standards like EIP712.
3. We use internals in modifiers to save code size.
4. Zero initialization is now handled by the optimizer and is more explicit.
5. Correct me if I'm wrong, but > uses just a GT opcode, whereas != uses EQ and NOT, so 3 extra gas.
6. When a treasury fee is implemented, having that additional check before calling the currency will be less efficient, it's assumed that for most of the protocol's existence, the treasury fee will be non-zero.

[Zer0dot]

Actually on the treasury fee point, after some discussion, I think it may be valid to implement, linking issue #62 in relation to this