Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas Optimizations #73

Open
code423n4 opened this issue May 1, 2022 · 1 comment
Open

Gas Optimizations #73

code423n4 opened this issue May 1, 2022 · 1 comment
Assignees
Labels
bug Something isn't working G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

Gas Report

Table of Contents

Comparisons with zero for unsigned integers

IMPACT

> 0 is less gas efficient than != 0 if you enable the optimizer at 10k AND you’re in a require statement.
Detailed explanation with the opcodes here

PROOF OF CONCEPT

Instances include:

AaveV3YieldSource.sol

AaveV3YieldSource.sol:179: require(decimals_ > 0, "AaveV3YS/decimals-gt-zero");
AaveV3YieldSource.sol:233: require(_shares > 0, "AaveV3YS/shares-gt-zero");

TOOLS USED

Manual Analysis

MITIGATION

Replace > 0 with != 0

Constructor parameters should be avoided when possible

IMPACT

Constructor parameters are expensive. The contract deployment will be cheaper in gas if they are hard coded instead of using constructor parameters.

PROOF OF CONCEPT

Instances include:

AaveV3YieldSource.sol

AaveV3YieldSource.sol:160 IAToken _aToken
AaveV3YieldSource.sol:161 IRewardsController _rewardsController
AaveV3YieldSource.sol:162 IPoolAddressesProviderRegistry _poolAddressesProviderRegistry
AaveV3YieldSource.sol:165 uint8 decimals_

TOOLS USED

Manual Analysis

MITIGATION

Hardcode aToken, rewardsController, poolAddressesProviderRegistry and _decimals with their values instead of writing them during contract deployment with constructor parameters.

Custom Errors

IMPACT

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) while providing the same amount of information, as explained here

Custom errors are defined using the error statement

PROOF OF CONCEPT

Instances include:

AaveV3YieldSource.sol

AaveV3YieldSource.sol:168: require(address(_aToken) != address(0), "AaveV3YS/aToken-not-zero-address");
AaveV3YieldSource.sol:171: require(address(_rewardsController) != address(0), "AaveV3YS/RC-not-zero-address");
AaveV3YieldSource.sol:174: require(address(_poolAddressesProviderRegistry) != address(0), "AaveV3YS/PR-not-zero-address");
AaveV3YieldSource.sol:177: require(_owner != address(0), "AaveV3YS/owner-not-zero-address");
AaveV3YieldSource.sol:179: require(decimals_ > 0, "AaveV3YS/decimals-gt-zero");
AaveV3YieldSource.sol:233: require(_shares > 0, "AaveV3YS/shares-gt-zero");
AaveV3YieldSource.sol:276: require(_to != address(0), "AaveV3YS/payee-not-zero-address");
AaveV3YieldSource.sol:337: require(address(_token) != address(aToken), "AaveV3YS/forbid-aToken-transfer");
AaveV3YieldSource.sol:349: require(_token != address(aToken), "AaveV3YS/forbid-aToken-allowance");

TOOLS USED

Manual Analysis

MITIGATION

Replace require and revert statements with custom errors.

For instance:

Replace

require(decimals_ > 0, "AaveV3YS/decimals-gt-zero");

with

if (decimals_ == 0) {
		revert ErrorDecimalNull(decimals_);
}

and define the custom error in the contract

error ErrorDecimalNull(uint8 decimals_);

Inline functions

PROBLEM

When we define internal functions to perform computation:

  • The contract’s code size gets bigger
  • the function call consumes more gas than executing it as an inlined function (part of the code, without the function call)

When it does not affect readability, it is recommended to inline functions in order to save gas

PROOF OF CONCEPT

Instances include:

AaveV3YieldSource.sol

AaveV3YieldSource.sol: 348: function _requireNotAToken(address _token)
AaveV3YieldSource.sol: 380: function _tokenAddress()

TOOLS USED

Manual Analysis

MITIGATION

Inline these functions where they are called:

AaveV3YieldSource.sol:183
AaveV3YieldSource.sol:212
AaveV3YieldSource.sol:235
AaveV3YieldSource.sol:252
AaveV3YieldSource.sol:301
AaveV3YieldSource.sol:320

Safemath library redundant

PROBLEM

As of Solidity 0.8.0, overflow and underflow checks are performed automatically. Using the SafeMath library methods for subtractions and additions is hence superfluous and cost additional gas upon deployment and function calls

PROOF OF CONCEPT

Instances include:

AaveV3YieldSource.sol

AaveV3YieldSource.sol:262:   uint256 _balanceDiff = _afterBalance.sub(_beforeBalance);

TOOLS USED

Manual Analysis

MITIGATION

Replace the sub() call with the regular - operation

Tight Variable Packing

PROBLEM

Solidity contracts have contiguous 32 bytes (256 bits) slots used in storage.
By arranging the variables, it is possible to minimize the number of slots used within a contract's storage and therefore reduce deployment costs.

uint8 variables are each of 1 byte size (way less than 32 bytes). However, here it takes up a whole 32 bytes slot, as it is followed in storage by a uint256 variable.

By rearranging the storage variables, we can save one slot

PROOF OF CONCEPT

Instances include:

Funding.sol

AaveV3YieldSource.sol:136-145
uint8 private immutable _decimals;

/**
  * @dev Aave genesis market PoolAddressesProvider's ID.
  * @dev This variable could evolve in the future if we decide to support other markets.
  */
uint256 private constant ADDRESSES_PROVIDER_ID = uint256(0);

/// @dev PoolTogether's Aave Referral Code
uint16 private constant REFERRAL_CODE = uint16(188);

TOOLS USED

Manual Analysis

MITIGATION

Place REFERRAL_CODE after _decimals to save one storage slot

uint8 private immutable _decimals;
+uint16 private constant REFERRAL_CODE = uint16(188);
/**
  * @dev Aave genesis market PoolAddressesProvider's ID.
  * @dev This variable could evolve in the future if we decide to support other markets.
  */
uint256 private constant ADDRESSES_PROVIDER_ID = uint256(0);
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels May 1, 2022
code423n4 added a commit that referenced this issue May 1, 2022
@PierrickGT PierrickGT self-assigned this May 3, 2022
@PierrickGT
Copy link
Member

Comparisons with zero for unsigned integers
Safemath library redundant

Duplicate of #11

Constructor parameters should be avoided when possible

This parameters need to be passed to the initialized function.

Custom Errors

Duplicate of #13

Inline functions

Duplicate of #19

Tight Variable Packing

The compiler is smart enough to properly allocate memory. I didn't notice any gas saving when testing the change proposed by the warden.

@PierrickGT PierrickGT added the duplicate This issue or pull request already exists label May 3, 2022
@JeeberC4 JeeberC4 removed the duplicate This issue or pull request already exists label May 6, 2022
@JeeberC4 JeeberC4 reopened this May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization)
Projects
None yet
Development

No branches or pull requests

3 participants