Platform: Code4rena
Start Date: 30/03/2022
Pot Size: $30,000 USDC
Total HM: 21
Participants: 38
Period: 3 days
Judge: Michael De Luca
Total Solo HM: 10
Id: 104
League: ETH
Rank: 26/38
Findings: 2
Award: $143.33
🌟 Selected for report: 0
🚀 Solo Findings: 0
86.1007 USDC - $86.10
2022-03-joyn 1 Lock pragmas to specific compiler version. Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.
For example,
pragma solidity 0.8.0;
2 delete unused modifier. modifier onlyUnInitialized in CoreCollection is never used in the contract. You can delete it if you don’t use it.
Delete it.
3 modifier onlyInitialized used only one time in the contract. modifier onlyInitialized is used only in mintToken, so you can write this require statement directly in the function. From the view of gas, both have the same result.
Add the following code to the top of mintToken and remove onlyInitialized from the function.
require(initialized, "CoreCollection: Not initialized");
4 modifier tokenExists is never used. You can delete it.
Delete it.
5 startingIndexBlock is never used. startingIndexBlock will be set in setStartingIndex. However, this state variable will be never used. If you don’t use it, you can delete it.
https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L26 https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L230
Delete them.
6 wrong description in comment of transferSplitAsset. Here you try to transfer ERC20 tokens. No ETH.
https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/Splitter.sol#L235
For example,
// Try to transfer splitAsset to the given recipient.
7 delete an unused function. Ths following function is never used. I guess this function is there because of the fork. You can delete it.
https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/Splitter.sol#L248-L257
Delete it.
8 event TransferETH has the wrong event name. The following line defines event TransferETH. However, ETH will be not transferred in transferSplitAsset. The event name and comment for it must be changed to avoid confusion.
https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/Splitter.sol#L17-L18
For example,
event TransferSplitAsset();
9 missing input validation in setPlatformFee. In setPlatformFee input address must be checked if the address is empty or not.
https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/SplitFactory.sol#L120-L125
Add the following code to the beginning of the function.
require(_vault != address(0), 'Invalid vault');
10 delete unused import statement.
https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreFactory.sol#L6
11 delete unused import statement.
https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L8
57.2279 USDC - $57.23
2022-03-joyn gas optimization
1 Use initial value for string. You set default value for HASHED_PROOF. To save deployment gas cost you can use initial value for string.
https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L27
string public HASHED_PROOF;
Avg deployment from 3070877 to 3067973 with this change
2 modifier onlyValidSupply used only one time in the contract. modifier onlyValidSupply is used only in initialize, so you can write this require statement directly in the function to save gas.
Add the following code to the top of initialize and remove from onlyValidSupply the function.
require(
_maxSupply > 0,
"CoreCollection: Max supply should be greater than 0"
);
Avg deployment from 3070877 to 3070313 with this change according to hardhat-gas-reporter
3 use != 0 instead of > 0 in if sentence. The following line use > 0 in if sentence of mintToken. != is cheaper than > 0.
if (mintFee != 0) {}
4 Use cache for royaltyVaultInitialized() in _handlePayment. royaltyVaultInitialized()is used twice in _handlePayment, so it will be cheaper to use cache royaltyVaultInitialized().
function _handlePayment(uint256 _amount) internal { bool vaultInitialized = royaltyVaultInitialized(); address recipient = vaultInitialized ? royaltyVault : address(this); payableToken.transferFrom(msg.sender, recipient, _amount); emit NewPayment(msg.sender, recipient, _amount, vaultInitialized); }
mintToken Avg from 228191 to 228124 with this change according to hardhat-gas-reporter
5 delete == true. In require you don’t use == true to check return value of IERC20().transfer and can save a little bit gas cost.
https://github.com/code-423n4/2022-03-joyn/blob/main/royalty-vault/contracts/RoyaltyVault.sol#L44 https://github.com/code-423n4/2022-03-joyn/blob/main/royalty-vault/contracts/RoyaltyVault.sol#L48 https://github.com/code-423n4/2022-03-joyn/blob/main/royalty-vault/contracts/RoyaltyVault.sol#L52-L55
require(IERC20().transfer, “Failed to transfer royalty Asset to”);
6 input validation must be checked earlier in the following function. royaltyAmount as input for incrementWindow must be checked at the top of the function to save gas cost in case of reverting with this validation.
https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/Splitter.sol#L164 Add this require sentence to the beginning of the function.
7 code duplication. splitAsset and royaltyAsset are always the same, so you can use a common variable for these variables. For example, you create new a variable transactionAsset instead of these variables. When you call createSplit, createSplitProxy will be called first and after that createVaultProxy will be called, so you can delete transactionAsset in createVaultProxy . You need to change the constructor of SplitProxy and ProxyVault too.
https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/SplitFactory.sol#L20-L21 https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/SplitFactory.sol#L86-L87 https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/SplitFactory.sol#L108-L109 https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/SplitFactory.sol#L162 https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/SplitFactory.sol#L172 https://github.com/code-423n4/2022-03-joyn/blob/main/royalty-vault/contracts/ProxyVault.sol#L19 https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/SplitProxy.sol#L20
address public transactionAsset;
8 set royaltyVault immutable. There is no setter for royaltyVault in ProxyVault, so royaltyVault in ProxyVault can be immutable.
https://github.com/code-423n4/2022-03-joyn/blob/main/royalty-vault/contracts/ProxyVault.sol#L9
address internal immutable royaltyVault;
9 use unchecked and prefix in for loop in the following loop.
https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L279 https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreFactory.sol https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/Splitter.sol#L50 https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/Splitter.sol#L274
for (uint256 i; i < length;) { unchecked { ++i; } }
10 use calldata instead of memory in createProject. The following inputs are memory. You can use calldata instated of memory to save gas.
string memory _projectId, Collection[] memory _collections
createProject() Avg from 602069 to 601329 only with this change according to hardhat-gas-reporter
11 use calldata instead of memory in addCollection. The following inputs are memory. You can use calldata instated of memory to save gas.
string calldata _projectId, Collection calldata _collection
addCollection() Avg from 442267 to 442141 only with this change according to hardhat-gas-reporter
12 use calldata instead of memory in getProject.
https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreFactory.sol#L128