code-423n4 / 2022-03-joyn-findings

4 stars 1 forks source link

Gas Optimizations #113

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

gas

1 Unused onlyUnInitialized modifier

https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L46 The onlyUnInitialized was never called everywhere. Just remove the modifier

2 Using calldata to store string as a parameter

https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L79-L81 https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L122-L123 By using calldata to store _collectionName _collectionSymbol and _collectionURI can save gas when running initialize()

3 Using != instead of >

https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L146 https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L305 Using != is more effective for gas improvement

4 Gas improvement by not setting uint = 0

https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L279 Uint has a default value 0. By just declare it without setting the value can save gas

5 Prefix increment and unchecked for gas improvement

https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L279 Using ++i for increment instead of i++ can save gas. Then use unchecked to do the increment:

for (uint256 i = 0; i < _amount;) {
            uint256 tokenId = mint(_to);
            if (_isClaim) {
                emit NewClaim(msg.sender, _to, tokenId);
            }
Unchecked{
    ++i
}
        }

6 Don't set string default value

https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L27 Declaring HASHED_PROOF without setting it the value can save gas

7 Remove _beforeTokenTransfer call in the _beforeTokenTransfer function

https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L301 _beforeTokenTransfer function (parent contract) is an empty function. Just remove L301 to save gas

8 Using custom error instead of revert string

Custom error is declared with error statement. Then replace all the require(condition, 'revertString') with if(condition) revert(error)

9 Using calldata to store array parameter value

https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreFactory.sol#L72 By using calldata to store _collections can save gas

10 Use storage to store _collection

https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreFactory.sol#L80 If var amount inside struct >= struct called times, using storage is way cheaper:

   Collection storage _collection = _collections[I]; //@audit-info change memory to storage

Same thing for project at L93

11 Better implementation of calling SafeERC20.function

https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreMultiSig.sol#L10 By removing L10, and calling SafeERC20.function: L10

SafeERC20.safeTransfer(IERC20(token), to, amount)

Can save 15 gas per call

12 Tight variable packing in Collection struct

https://github.com/code-423n4/2022-03-joyn/blob/c9297ccd925ebb2c44dbc6eaa3effd8db5d2368a/core-contracts/contracts/utils/structs/Collection.sol By placing isForSale bool under payableToken address can save 1 slot

13 Using multiple require is more effective than &&

https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/MultiSigWallet.sol#L99 Change the code to:

            require(!isOwner[_owners[i]]);
            require(!isOwner[_owners[i] != address(0));

14 Tight variable packing in Transaction struct

https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/MultiSigWallet.sol#L32-L37 By moving destination location above executed can save 1 slot

    struct Transaction {
        uint256 value;
        bytes data;
    address destination;//@audit-info move here
        bool executed;
    }

15 Using delete statement instead of setting value to false

https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/MultiSigWallet.sol#L152 By using delete to set mapping to default value (false) can save gas:

Delete isOwner[owner];

Also: https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/MultiSigWallet.sol#L206

deluca-mike commented 2 years ago

Funnily enough point 1 was an indication of #4