Joyn contest - rfa's results

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