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

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

Gas Optimizations #265

code423n4 opened this issue Jun 19, 2022 · 2 comments
Labels
bug Something isn't working duplicate This issue or pull request already exists G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

1. Cache array length in loops instead of computing length in every iteration

While looping through array, the array length can be cached to save gas instead of computing length in each array iteration.

for eg.

uint256 len = modules.length;
for(uint256 i = 0; i < len; i++) {
  ...
}
> grep -rn './core' -e 'for.*[.]length'
./core/connext/facets/StableSwapFacet.sol:415:    for (uint8 i = 0; i < _pooledTokens.length; i++) {
./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/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/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/helpers/ConnextPriceOracle.sol:176:    for (uint256 i = 0; i < tokenAddresses.length; i++) {
./core/connext/helpers/StableSwap.sol:81:    for (uint8 i = 0; i < _pooledTokens.length; i++) {

2. Prefix increment (++i) is cheaper than postfix increment (i++)

Using prefix increment is saves small amount of gas than postfix increment because it returns the updated value hence doesn't requires to store intermediate value. This can be more significant in loops where this operation is done multiple times.

for eg.

// before
for(uint256 i = 0; i < len; i++) {
  ...
}

// Replace with
for(uint256 i = 0; i < len; ++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/ConnextPriceOracle.sol:176:    for (uint256 i = 0; i < tokenAddresses.length; i++) {
./core/connext/helpers/StableSwap.sol:81:    for (uint8 i = 0; i < _pooledTokens.length; i++) {
...

3.Cache compute of address(this).balance in ternary operator to save gas instead of evaluating it twice

sponsoredFee = sponsoredFee > address(this).balance ? address(this).balance : sponsoredFee; 
@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
@liu-zhipeng
Copy link
Collaborator

1, 2 : duplicated
3: fixed

@liu-zhipeng liu-zhipeng added the duplicate This issue or pull request already exists label Jun 30, 2022
@itsmetechjay itsmetechjay reopened this Jul 1, 2022
@0xleastwood
Copy link
Collaborator

Useful optimisations but also found by many other wardens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists G (Gas Optimization)
Projects
None yet
Development

No branches or pull requests

4 participants