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

Open
code423n4 opened this issue Feb 12, 2022 · 3 comments
Open

Gas Optimizations #64

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

Comments

@code423n4
Copy link
Contributor

##NestedFinanceGasFindings
1--
-using multiple require() is gas saving
https://github.com/code-423n4/2022-02-nested/blob/main/contracts/NestedFactory.sol#L54-L62
instead of using &&, using multiple require is gas saving.

require(address(_nestedAsset) != address(0));
require(address(_nestedRecords) != address(0));
...

2--
-better for() implementation
https://github.com/code-423n4/2022-02-nested/blob/main/contracts/NestedFactory.sol#L113
replace i++ to ++i and dont set the i value because the default is already 0. Its cost less gas usage

for (uint256 i; i < operatorsLength; ++i)

3--
-Better way of using SafeERC20 lib
https://github.com/code-423n4/2022-02-nested/blob/main/contracts/NestedFactory.sol#L19
By calling SafeERC20.function directly and removing line 19 can save 15 gas per call:

SafeERC20.safeTransfer(_token, owner(), amount);

SafeErc20.function was called 8 times in this contract. Also very good to implemented at NestedReserve.sol
4--
-using storage instead of caching struct/array data can save gas
https://github.com/code-423n4/2022-02-nested/blob/main/contracts/NestedFactory.sol#L205
using storage can save gas

address[] storage tokens = nestedRecords.getAssetTokens(_nftId);

tokens is called once at destroy() before it chaced to tokensLength as tokens.lenght so reading from storage is cheaper than using memory
5--
-use calldata to store _weights & _account
https://github.com/code-423n4/2022-02-nested/blob/main/contracts/FeeSplitter.sol#L89-L90
change memory to calldata

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Feb 12, 2022
code423n4 added a commit that referenced this issue Feb 12, 2022
@maximebrugel
Copy link
Collaborator

1 (Disputed)

Not the case. From 4966507 to 4970427

2 (Disputed)

Already in first audit : code-423n4/2021-11-nested-findings#25

3 (Duplicated)

#55

4 (Disputed)

Compilation error address[] storage tokens = nestedRecords.getAssetTokens(_nftId); you can't convert to storage like this.

5 (Disputed)

Data location must be "storage" or "memory" for constructor parameter.

@maximebrugel maximebrugel added the duplicate This issue or pull request already exists label Feb 17, 2022
@harleythedogC4
Copy link
Collaborator

My personal judgements:

  1. "using multiple require() is gas saving". This was confirmed by the sponsor in Gas Optimizations #55, so it should be valid here too (although it is a very small difference in gas). Valid and small optimization.
  2. "better for() implementation". Agree with sponsor. Invalid.
  3. "Better way of using SafeERC20 lib". Valid and small optimization.
  4. "using storage instead of caching struct/array data can save gas". Example gives compilation error, Invalid.
  5. "use calldata to store _weights & _account". As sponsor says. Invalid.

@harleythedogC4
Copy link
Collaborator

Now, here is the methodology I used for calculating a score for each gas report. I first assigned each submission to be either small-optimization (1 point), medium-optimization (5 points) or large-optimization (10 points), depending on how useful the optimization is. The score of a gas report is the sum of these points, divided by the maximum number of points achieved by a gas report. This maximum number was 10 points, achieved by #67.

The number of points achieved by this report is 2 points.
Thus the final score of this gas report is (2/10)*100 = 20.

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

3 participants