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 #274

Open
code423n4 opened this issue Jun 19, 2022 · 0 comments
Open

Gas Optimizations #274

code423n4 opened this issue Jun 19, 2022 · 0 comments
Labels
bug Something isn't working G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

Use != 0 instead of > 0 when comparing unsigned integers

!= 0 will do the same as > 0 for unsigned integers, but != 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled.

Instances include :

core/connext/libraries/SwapUtils.sol:845:      require(v.totalSupply != 0 || amounts[i] > 0, "Must supply all tokens in pool");
core/connext/libraries/AmplificationUtils.sol:86:    require(futureA_ > 0 && futureA_ < MAX_A, "futureA_ must be > 0 and < MAX_A");
core/connext/libraries/LibDiamond.sol:121:    require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
core/connext/libraries/LibDiamond.sol:139:    require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
core/connext/libraries/LibDiamond.sol:158:    require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
core/connext/libraries/LibDiamond.sol:226:      require(_calldata.length > 0, "LibDiamondCut: _calldata is empty but _init is not address(0)");
core/connext/libraries/LibDiamond.sol:247:    require(contractSize > 0, _errorMessage);
core/connext/helpers/ConnextPriceOracle.sol:150:    require(baseTokenPrice > 0, "invalid base token");

Recommendation

It is recommended to replace > 0 with != 0, as they do the same thing for unsigned integers, and '!= 0' costs less gas compared to > 0 in require statements with the optimizer enabled, also enable the optimizer.

For example :

core/connext/helpers/ConnextPriceOracle.sol:150:    require(baseTokenPrice != 0, "invalid base token");

Don't explicitly initialize variables with the default value

Uninitialized variables are assigned with the default value of their type, initializing a variable with its default value costs unnecessary gas.

Instances include :

core/relayer-fee/libraries/RelayerFeeMessage.sol:81:    for (uint256 i = 0; i < length; ) {
core/shared/Version.sol:9:  uint8 public constant VERSION = 0;
core/connext/facets/VersionFacet.sol:16:  uint8 internal immutable _version = 0;
core/connext/facets/BridgeFacet.sol:68:  uint16 public constant AAVE_REFERRAL_CODE = 0;
core/connext/facets/StableSwapFacet.sol:415:    for (uint8 i = 0; i < _pooledTokens.length; i++) {
core/connext/libraries/Encoding.sol:22:    for (uint8 i = 0; i < 10; i += 1) {
core/connext/libraries/SwapUtils.sol:205:    for (uint256 i = 0; i < xp.length; i++) {
core/connext/libraries/SwapUtils.sol:254:    for (uint256 i = 0; i < numTokens; i++) {
core/connext/libraries/SwapUtils.sol:268:    for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
core/connext/libraries/SwapUtils.sol:289:    for (uint256 i = 0; i < numTokens; i++) {
core/connext/libraries/SwapUtils.sol:300:    for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
core/connext/libraries/SwapUtils.sol:302:      for (uint256 j = 0; j < numTokens; j++) {
core/connext/libraries/SwapUtils.sol:344:    for (uint256 i = 0; i < numTokens; i++) {
core/connext/libraries/SwapUtils.sol:405:    for (uint256 i = 0; i < numTokens; i++) {
core/connext/libraries/SwapUtils.sol:425:    for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
core/connext/libraries/SwapUtils.sol:558:    for (uint256 i = 0; i < balances.length; i++) {
core/connext/libraries/SwapUtils.sol:591:    for (uint256 i = 0; i < balances.length; i++) {
core/connext/libraries/SwapUtils.sol:844:    for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:869:      for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:924:    for (uint256 i = 0; i < amounts.length; i++) {
core/connext/libraries/SwapUtils.sol:1014:      for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:1019:      for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:1039:    for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:1055:    for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/helpers/Multicall.sol:16:    for (uint256 i = 0; i < calls.length; i++) {
core/connext/helpers/StableSwap.sol:81:    for (uint8 i = 0; i < _pooledTokens.length; i++) {
core/connext/helpers/ConnextPriceOracle.sol:176:    for (uint256 i = 0; i < tokenAddresses.length; i++) {

Recommendation

It is recommended to initialize variables without assigning them the default value, for example :

core/connext/helpers/ConnextPriceOracle.sol:176:    for (uint256 i; i < tokenAddresses.length; i++) {

Cache array length outside of for 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.

Instances include :

core/connext/facets/RelayerFacet.sol:140:    for (uint256 i; i < _transferIds.length; ) {
core/connext/facets/RelayerFacet.sol:164:    for (uint256 i; i < _transferIds.length; ) {
core/connext/facets/StableSwapFacet.sol:415:    for (uint8 i = 0; i < _pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:205:    for (uint256 i = 0; i < xp.length; i++) {
core/connext/libraries/SwapUtils.sol:558:    for (uint256 i = 0; i < balances.length; i++) {
core/connext/libraries/SwapUtils.sol:591:    for (uint256 i = 0; i < balances.length; i++) {
core/connext/libraries/SwapUtils.sol:844:    for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:869:      for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:924:    for (uint256 i = 0; i < amounts.length; i++) {
core/connext/libraries/SwapUtils.sol:1014:      for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:1019:      for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:1039:    for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:1055:    for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/LibDiamond.sol:104:    for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) {
core/connext/libraries/LibDiamond.sol:129:    for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
core/connext/libraries/LibDiamond.sol:147:    for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
core/connext/libraries/LibDiamond.sol:162:    for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
core/connext/helpers/Multicall.sol:16:    for (uint256 i = 0; i < calls.length; i++) {
core/connext/helpers/StableSwap.sol:81:    for (uint8 i = 0; i < _pooledTokens.length; i++) {
core/connext/helpers/ConnextPriceOracle.sol:176:    for (uint256 i = 0; i < tokenAddresses.length; i++) {

Recommendation

It is recommended to cache the array length on a variable before running the loop, then it doesn't need to read the length on every iteration, which cost gas, for example :

uint256 len = tokenAddresses.length;
core/connext/helpers/ConnextPriceOracle.sol:176:    for (uint256 i = 0; i < len; i++) {

If possible, use prefix increment instead of postfix increment

Prefix increment ++i returns the updated value after it's incremented and postfix increment i++ returns the original value then increments it. Prefix increment costs less gas compared to postfix increment.

Instances includes :

core/relayer-fee/libraries/RelayerFeeMessage.sol:85:        i++;
core/connext/facets/RelayerFacet.sol:144:        i++;
core/connext/facets/RelayerFacet.sol:168:        i++;
core/connext/facets/BridgeFacet.sol:613:          i++;
core/connext/facets/BridgeFacet.sol:684:          i++;
core/connext/facets/BridgeFacet.sol:799:            i++;
core/connext/facets/DiamondLoupeFacet.sol:31:    for (uint256 i; i < numFacets; i++) {
core/connext/facets/StableSwapFacet.sol:415:    for (uint8 i = 0; i < _pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:205:    for (uint256 i = 0; i < xp.length; i++) {
core/connext/libraries/SwapUtils.sol:254:    for (uint256 i = 0; i < numTokens; i++) {
core/connext/libraries/SwapUtils.sol:268:    for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
core/connext/libraries/SwapUtils.sol:289:    for (uint256 i = 0; i < numTokens; i++) {
core/connext/libraries/SwapUtils.sol:300:    for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
core/connext/libraries/SwapUtils.sol:302:      for (uint256 j = 0; j < numTokens; j++) {
core/connext/libraries/SwapUtils.sol:344:    for (uint256 i = 0; i < numTokens; i++) {
core/connext/libraries/SwapUtils.sol:405:    for (uint256 i = 0; i < numTokens; i++) {
core/connext/libraries/SwapUtils.sol:425:    for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
core/connext/libraries/SwapUtils.sol:558:    for (uint256 i = 0; i < balances.length; i++) {
core/connext/libraries/SwapUtils.sol:591:    for (uint256 i = 0; i < balances.length; i++) {
core/connext/libraries/SwapUtils.sol:844:    for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:869:      for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:924:    for (uint256 i = 0; i < amounts.length; i++) {
core/connext/libraries/SwapUtils.sol:1014:      for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:1019:      for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:1039:    for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/SwapUtils.sol:1055:    for (uint256 i = 0; i < pooledTokens.length; i++) {
core/connext/libraries/LibDiamond.sol:104:    for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) {
core/connext/libraries/LibDiamond.sol:129:    for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
core/connext/libraries/LibDiamond.sol:134:      selectorPosition++;
core/connext/libraries/LibDiamond.sol:147:    for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
core/connext/libraries/LibDiamond.sol:153:      selectorPosition++;
core/connext/libraries/LibDiamond.sol:162:    for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
core/connext/helpers/Multicall.sol:16:    for (uint256 i = 0; i < calls.length; i++) {
core/connext/helpers/StableSwap.sol:81:    for (uint8 i = 0; i < _pooledTokens.length; i++) {
core/connext/helpers/ConnextPriceOracle.sol:176:    for (uint256 i = 0; i < tokenAddresses.length; i++) {

Recommendation

It is recommended to use prefix increment instead of postfix one when the return value is not needed, as both of them will give the same result and prefix increment costs less gas.

For example :

core/connext/libraries/SwapUtils.sol:205:    for (uint256 i = 0; i < xp.length; ++i) {

Don't use long revert strings

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

Instances includes :

core/connext/helpers/OZERC20.sol:185:    require(_sender != address(0), "ERC20: transfer from the zero address");
core/connext/helpers/OZERC20.sol:186:    require(_recipient != address(0), "ERC20: transfer to the zero address");
core/connext/helpers/OZERC20.sol:205:    require(_account != address(0), "ERC20: mint to the zero address");
core/connext/helpers/OZERC20.sol:226:    require(_account != address(0), "ERC20: burn from the zero address");
core/connext/helpers/OZERC20.sol:253:    require(_owner != address(0), "ERC20: approve from the zero address");
core/connext/helpers/OZERC20.sol:254:    require(_spender != address(0), "ERC20: approve to the zero address");
core/connext/libraries/LibDiamond.sol:66:    require(msg.sender == diamondStorage().contractOwner, "LibDiamond: Must be contract owner");
core/connext/libraries/LibDiamond.sol:121:    require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
core/connext/libraries/LibDiamond.sol:123:    require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)");
core/connext/libraries/LibDiamond.sol:132:      require(oldFacetAddress == address(0), "LibDiamondCut: Can't add function that already exists");
core/connext/libraries/LibDiamond.sol:139:    require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
core/connext/libraries/LibDiamond.sol:141:    require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)");
core/connext/libraries/LibDiamond.sol:150:      require(oldFacetAddress != _facetAddress, "LibDiamondCut: Can't replace function with same function");
core/connext/libraries/LibDiamond.sol:158:    require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
core/connext/libraries/LibDiamond.sol:161:    require(_facetAddress == address(0), "LibDiamondCut: Remove facet address must be address(0)");
core/connext/libraries/LibDiamond.sol:191:    require(_facetAddress != address(0), "LibDiamondCut: Can't remove function that doesn't exist");
core/connext/libraries/LibDiamond.sol:193:    require(_facetAddress != address(this), "LibDiamondCut: Can't remove immutable function");
core/connext/libraries/LibDiamond.sol:224:      require(_calldata.length == 0, "LibDiamondCut: _init is address(0) but_calldata is not empty");
core/connext/libraries/LibDiamond.sol:226:      require(_calldata.length > 0, "LibDiamondCut: _calldata is empty but _init is not address(0)");
core/connext/libraries/SwapUtils.sol:784:    require(dy <= self.balances[tokenIndexTo], "Cannot get more than pool balance");
core/connext/libraries/SwapUtils.sol:823:    require(amounts.length == pooledTokens.length, "Amounts must match pooled tokens");
core/connext/libraries/SwapUtils.sol:917:    require(minAmounts.length == pooledTokens.length, "minAmounts must match poolTokens");

Recommendation

It is recommended to use error code and providing a reference to the error code instead of a long revert string., for example :

// Link to the reference of error codes
core/connext/libraries/SwapUtils.sol:917:    require(minAmounts.length == pooledTokens.length, "ERR");

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Jun 19, 2022
code423n4 added a commit that referenced this issue Jun 19, 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

1 participant