Joyn contest - IllIllI's results

Low Risk Issues

Unused receive() function will lock Ether in contract

If the intention is for the Ether to be used, the function should emit or call another function, otherwise it should revert

1. File: royalty-vault/contracts/ProxyVault.sol (line 47)

    receive() external payable {}

2. File: splits/contracts/SplitProxy.sol (line 51)

    receive() external payable {}

Using payable with the fallback() function will lock Ether in the contract

The Ether is never used by any of the proxied contracts - only WETH is

1. File: royalty-vault/contracts/ProxyVault.sol (line 28)

    fallback() external payable {

2. File: splits/contracts/SplitProxy.sol (line 24)

    fallback() external payable {

Missing checks for address(0x0) when assigning values to address state variables

1. File: core-contracts/contracts/CoreProxy.sol (line 12)

        _implement = _imp;

2. File: core-contracts/contracts/CoreFactory.sol (line 28)

    collection = _collection;

3. File: core-contracts/contracts/CoreFactory.sol (line 29)

    splitFactory = _splitFactory;

4. File: splits/contracts/SplitFactory.sol (line 61)

    royaltyVault = _royaltyVault;

5. File: splits/contracts/SplitFactory.sol (line 86)

    splitAsset = _splitAsset;

6. File: splits/contracts/SplitFactory.sol (line 87)

    royaltyAsset = _splitAsset;

7. File: splits/contracts/SplitFactory.sol (line 108)

    splitAsset = _splitAsset;

8. File: splits/contracts/SplitFactory.sol (line 109)

    royaltyAsset = _splitAsset;

9. File: splits/contracts/SplitFactory.sol (line 169)

    splitterProxy = splitProxy;

Misleading revert() strings and variable names

The contract is not specific to either ETH or WETH. This appears to be left over from forking from mirror-xyz/splits

1. File: splits/contracts/Splitter.sol (line 237)

        require(didSucceed, "Failed to transfer ETH");

2. File: splits/contracts/Splitter.sol (line 150)

        uint256 wethBalance;

No fees may be sent if payableToken does not have enough decimals and amounts are small

There are tokens such as ERC884 tokens, which are ERC20-compatible and are required to have zero decimals. The fix would be to require a minimum number of decimals.

1. File: splits/contracts/Splitter.sol (line 103)

        scaledAmount = (amount * scaledPercent) / (10000);

Non-critical Issues

public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public.

1. File: core-contracts/contracts/CoreCollection.sol (line 244)

    function baseURI() public view returns (string memory) {

2. File: splits/contracts/Splitter.sol (line 149)

    function incrementWindow(uint256 royaltyAmount) public returns (bool) {

constants should be defined rather than using magic numbers

1. File: royalty-vault/contracts/RoyaltyVault.sol (line 40)

        uint256 platformShare = (balanceOfVault * platformFee) / 10000;

2. File: splits/contracts/SplitFactory.sol (line 62)

    platformFee = 500; // 5%

3. File: splits/contracts/SplitFactory.sol (line 63)

    platformFeeRecipient = 0x70388C130222eae55a0527a2367486bF5D12d6e7;

4. File: splits/contracts/Splitter.sol (line 103)

        scaledAmount = (amount * scaledPercent) / (10000);

5. File: splits/contracts/Splitter.sol (line 223)

        return (amount * percent) / 100;

6. File: splits/contracts/Splitter.sol (line 255)

        (bool success, ) = to.call{value: value, gas: 30000}("");

Different versions of solidity used

1. File: royalty-vault/contracts/RoyaltyVault.sol (line 2)

pragma solidity ^0.8.4;

2. File: core-contracts/contracts/CoreProxy.sol (line 2)

pragma solidity ^0.8.0;

Use a more recent version of solidity

Use a solidity version of at least 0.8.12 to get string.concat() to be used instead of abi.encodePacked(<str>,<str>)

1. File: splits/contracts/Splitter.sol (line 2)

pragma solidity ^0.8.4;

Use a more recent version of solidity

Use a solidity version of at least 0.8.4 to get bytes.concat() instead of abi.encodePacked(<bytes>,<bytes>) Use a solidity version of at least 0.8.12 to get string.concat() instead of abi.encodePacked(<str>,<str>)

1. File: core-contracts/contracts/ERC721Claimable.sol (line 2)

pragma solidity ^0.8.0;

2. File: core-contracts/contracts/CoreCollection.sol (line 2)

pragma solidity ^0.8.0;

3. File: core-contracts/contracts/CoreFactory.sol (line 2)

pragma solidity ^0.8.0;

Variable names that consist of all capital letters should be reserved for const/immutable variables

If the variable needs to be different based on which class it comes from, a view/pure function should be used instead (e.g. like this).

1. File: core-contracts/contracts/CoreCollection.sol (line 27)

    string public HASHED_PROOF = "";

Non-library/interface files should use fixed compiler versions, not floating ones

1. File: core-contracts/contracts/CoreProxy.sol (line 2)

pragma solidity ^0.8.0;

2. File: core-contracts/contracts/CoreCollection.sol (line 2)

pragma solidity ^0.8.0;

3. File: core-contracts/contracts/CoreFactory.sol (line 2)

pragma solidity ^0.8.0;

4. File: royalty-vault/contracts/RoyaltyVault.sol (line 2)

pragma solidity ^0.8.4;

5. File: royalty-vault/contracts/ProxyVault.sol (line 2)

pragma solidity ^0.8.4;

6. File: splits/contracts/SplitFactory.sol (line 2)

pragma solidity ^0.8.4;

7. File: splits/contracts/SplitProxy.sol (line 2)

pragma solidity ^0.8.4;

8. File: splits/contracts/Splitter.sol (line 2)

pragma solidity ^0.8.4;

Typos

1. File: core-contracts/contracts/CoreCollection.sol (line 181)

     * @dev All revenue (Primary sales + royalties from secondardy sales) 

secondardy

2. File: core-contracts/contracts/CoreFactory.sol (line 139)

   * @notice Instanciates/Deploys a collection

Instanciates

3. File: splits/contracts/SplitFactory.sol (line 16)

  /**** Mmutable storage ****/

Mmutable

Grammar

1. File: core-contracts/contracts/CoreCollection.sol (line 134)

     * @param isClaim Whether the user want claim a token that has been airdropped to him or want to purchase the token

user want claim -> user wants to claim

NatSpec is incomplete

1. File: core-contracts/contracts/ERC721Claimable.sol (lines 55-62)

   * @param merkleProof Proof
   */
  function canClaim(
    address who,
    uint256 claimableAmount,
    uint256 claimedAmount,
    bytes32[] calldata merkleProof
  ) public view returns (bool) {

Missing: @return

2. File: core-contracts/contracts/CoreCollection.sol (lines 258-260)

     * @param _to Token recipient
     */
    function mint(address _to) private returns (uint256 tokenId) {

Missing: @return

3. File: core-contracts/contracts/CoreFactory.sol (lines 106-111)

   * @param _collection Collection that needs to be deployed
   */
  function addCollection(
    string memory _projectId,
    Collection memory _collection
  ) external onlyProjectOwner(_projectId) returns (address) {

Missing: @return

4. File: core-contracts/contracts/CoreFactory.sol (lines 140-145)

   * @param _collection Collection that needs to be deployed
   */
  function _createCollection(Collection memory _collection)
    private
    onlyAvailableCollection(_collection.id)
    returns (address)

Missing: @return

5. File: splits/contracts/SplitFactory.sol (lines 73-80)

   * @param _splitId The split identifier.
   */
  function createSplit(
    bytes32 _merkleRoot,
    address _splitAsset,
    address _collectionContract,
    string memory _splitId
  ) external onlyAvailableSplit(_splitId) returns (address splitProxy) {

Missing: @return

6. File: splits/contracts/SplitFactory.sol (lines 100-106)

   * @param _splitId The split identifier.
   */
  function createSplit(
    bytes32 _merkleRoot,
    address _splitAsset,
    string memory _splitId
  ) external onlyAvailableSplit(_splitId) returns (address splitProxy) {

Missing: @return

7. File: splits/contracts/Splitter.sol (lines 229-233)

     * @param value {uint256} Amount to transfer.
     */
    function transferSplitAsset(address to, uint256 value)
        private
        returns (bool didSucceed)

Missing: @return

Event is missing indexed fields

Each event should use three indexed fields if there are three or more fields

1. File: core-contracts/contracts/ERC721Payable.sol (lines 11-16)

  event NewPayment(
    address from,
    address to,
    uint256 amount,
    bool royaltyVaultPayment
  );

2. File: core-contracts/contracts/CoreCollection.sol (line 29)

    event ClaimInitialized(bytes32 root);

3. File: core-contracts/contracts/CoreCollection.sol (line 30)

    event NewCollectionMeta(string name, string symbol);

4. File: core-contracts/contracts/CoreCollection.sol (line 31)

    event NewClaim(address claimedBy, address to, uint256 tokenId);

5. File: core-contracts/contracts/CoreCollection.sol (line 32)

    event StartingIndexSet(uint256 index);

6. File: core-contracts/contracts/CoreCollection.sol (line 33)

    event RoyaltyVaultInitialized(address royaltyVault);

7. File: core-contracts/contracts/CoreCollection.sol (line 34)

    event NewHashedProof(string proof);

8. File: core-contracts/contracts/CoreCollection.sol (line 35)

    event NewWithdrawal(address to, uint256 amount);

9. File: core-contracts/contracts/CoreFactory.sol (line 15)

  event NewProject(string id, address creator);

10. File: core-contracts/contracts/CoreFactory.sol (lines 16-20)

  event NewCollection(
    string collectionId,
    address collection,
    string projectId
  );

11. File: royalty-vault/contracts/RoyaltyVault.sol (line 13)

    event RoyaltySentToSplitter(address indexed splitter, uint256 amount);

12. File: royalty-vault/contracts/RoyaltyVault.sol (lines 14-17)

    event FeeSentToPlatform(
        address indexed platformFeeRecipient,
        uint256 amount
    );

13. File: royalty-vault/contracts/RoyaltyVault.sol (line 18)

    event NewRoyaltyVaultPlatformFee(uint256 platformFee);

14. File: royalty-vault/contracts/RoyaltyVault.sol (line 19)

    event NewRoyaltyVaultPlatformFeeRecipient(address recipient);

15. File: splits/contracts/SplitFactory.sol (line 30)

  event SplitCreated(address indexed splitter, string splitId);

16. File: splits/contracts/SplitFactory.sol (lines 32-37)

  event VaultCreated(
    address indexed vault,
    address indexed splitter,
    uint256 platformFee,
    address platformFeeRecipient
  );

17. File: splits/contracts/Splitter.sol (lines 18-25)

    event TransferETH(
        // The account to which the transfer was attempted.
        address account,
        // The amount for transfer that was attempted.
        uint256 amount,
        // Whether or not the transfer succeeded.
        bool success
    );

18. File: splits/contracts/Splitter.sol (line 28)

    event WindowIncremented(uint256 currentWindow, uint256 fundsAvailable);

Non-exploitable reentrancies

Reentrancy in ProxyVault.constructor() (contracts/ProxyVault.sol#16-22):
	External calls:
	- royaltyVault = IVaultFactory(msg.sender).royaltyVault() (contracts/ProxyVault.sol#17)
	- splitterProxy = IVaultFactory(msg.sender).splitterProxy() (contracts/ProxyVault.sol#18)
	State variables written after the call(s):
	- splitterProxy = IVaultFactory(msg.sender).splitterProxy() (contracts/ProxyVault.sol#18)
Reentrancy in ProxyVault.constructor() (contracts/ProxyVault.sol#16-22):
	External calls:
	- royaltyVault = IVaultFactory(msg.sender).royaltyVault() (contracts/ProxyVault.sol#17)
	- splitterProxy = IVaultFactory(msg.sender).splitterProxy() (contracts/ProxyVault.sol#18)
	- royaltyAsset = IVaultFactory(msg.sender).royaltyAsset() (contracts/ProxyVault.sol#19)
	State variables written after the call(s):
	- royaltyAsset = IVaultFactory(msg.sender).royaltyAsset() (contracts/ProxyVault.sol#19)
Reentrancy in ProxyVault.constructor() (contracts/ProxyVault.sol#16-22):
	External calls:
	- royaltyVault = IVaultFactory(msg.sender).royaltyVault() (contracts/ProxyVault.sol#17)
	- splitterProxy = IVaultFactory(msg.sender).splitterProxy() (contracts/ProxyVault.sol#18)
	- royaltyAsset = IVaultFactory(msg.sender).royaltyAsset() (contracts/ProxyVault.sol#19)
	- platformFee = IVaultFactory(msg.sender).platformFee() (contracts/ProxyVault.sol#20)
	State variables written after the call(s):
	- platformFee = IVaultFactory(msg.sender).platformFee() (contracts/ProxyVault.sol#20)
Reentrancy in ProxyVault.constructor() (contracts/ProxyVault.sol#16-22):
	External calls:
	- royaltyVault = IVaultFactory(msg.sender).royaltyVault() (contracts/ProxyVault.sol#17)
	- splitterProxy = IVaultFactory(msg.sender).splitterProxy() (contracts/ProxyVault.sol#18)
	- royaltyAsset = IVaultFactory(msg.sender).royaltyAsset() (contracts/ProxyVault.sol#19)
	- platformFee = IVaultFactory(msg.sender).platformFee() (contracts/ProxyVault.sol#20)
	- platformFeeRecipient = IVaultFactory(msg.sender).platformFeeRecipient() (contracts/ProxyVault.sol#21)
	State variables written after the call(s):
	- platformFeeRecipient = IVaultFactory(msg.sender).platformFeeRecipient() (contracts/ProxyVault.sol#21)
Reentrancy in RoyaltyVaultFactory.createVault(address,address) (contracts/RoyaltyVaultFactory.sol#37-50):
	External calls:
	- vault = address(new ProxyVault()) (contracts/RoyaltyVaultFactory.sol#44-46)
	State variables written after the call(s):
	- delete royaltyAsset (contracts/RoyaltyVaultFactory.sol#49)
	- delete splitterProxy (contracts/RoyaltyVaultFactory.sol#48)

Reentrancy in RoyaltyVault.sendToSplitter() (contracts/RoyaltyVault.sol#31-61):
	External calls:
	- require(bool,string)(IERC20(royaltyAsset).transfer(splitterProxy,splitterShare) == true,Failed to transfer royalty Asset to splitter) (contracts/RoyaltyVault.sol#43-46)
	- require(bool,string)(ISplitter(splitterProxy).incrementWindow(splitterShare) == true,Failed to increment splitter window) (contracts/RoyaltyVault.sol#47-50)
	- require(bool,string)(IERC20(royaltyAsset).transfer(platformFeeRecipient,platformShare) == true,Failed to transfer royalty Asset to platform fee recipient) (contracts/RoyaltyVault.sol#51-57)
	Event emitted after the call(s):
	- FeeSentToPlatform(platformFeeRecipient,platformShare) (contracts/RoyaltyVault.sol#60)
	- RoyaltySentToSplitter(splitterProxy,splitterShare) (contracts/RoyaltyVault.sol#59)

require()/revert() strings longer than 32 bytes cost extra gas

1. File: core-contracts/contracts/ERC721Payable.sol (lines 21-24)

    require(
      !royaltyVaultInitialized(),
      'CoreCollection: Royalty Vault already initialized'
    );

2. File: core-contracts/contracts/ERC721Payable.sol (lines 29-32)

    require(
      royaltyVaultInitialized(),
      'CoreCollection: Royalty Vault not initialized'
    );

3. File: core-contracts/contracts/ERC721Claimable.sol (line 23)

    require(!claimableSet(), 'ERC721Claimable: Claimable is already set');

4. File: core-contracts/contracts/CoreCollection.sol (line 47)

        require(!initialized, "CoreCollection: Already initialized");

5. File: core-contracts/contracts/CoreCollection.sol (lines 52-55)

        require(
            _maxSupply > 0,
            "CoreCollection: Max supply should be greater than 0"
        );

6. File: core-contracts/contracts/CoreCollection.sol (line 146)

        require(amount > 0, "CoreCollection: Amount should be greater than 0");

7. File: core-contracts/contracts/CoreCollection.sol (lines 189-192)

        require(
            msg.sender == splitFactory || msg.sender == owner(),
            "CoreCollection: Only Split Factory or owner can initialize vault."
        );

8. File: core-contracts/contracts/CoreCollection.sol (lines 204-207)

        require(
            bytes(HASHED_PROOF).length == 0,
            "CoreCollection: Hashed Proof is set"
        );

9. File: core-contracts/contracts/CoreCollection.sol (lines 220-223)

        require(
            startingIndex == 0,
            "CoreCollection: Starting index is already set"
        );

10. File: core-contracts/contracts/CoreFactory.sol (lines 35-38)

    require(
      projects[_projectId].creator == address(0),
      'CoreFactory: Unavailable project id'
    );

11. File: core-contracts/contracts/CoreFactory.sol (lines 43-46)

    require(
      projects[_projectId].creator == msg.sender,
      'CoreFactory: Not an owner of the project'
    );

12. File: core-contracts/contracts/CoreFactory.sol (lines 51-54)

    require(
      collections[_collectionId] == address(0),
      'CoreFactory: Unavailable collection id'
    );

13. File: core-contracts/contracts/CoreFactory.sol (lines 74-77)

    require(
      _collections.length > 0,
      'CoreFactory: should have more at least one collection'
    );

14. File: royalty-vault/contracts/RoyaltyVault.sol (lines 34-37)

        require(
            balanceOfVault > 0,
            "Vault does not have enough royalty Asset to send"
        );

15. File: royalty-vault/contracts/RoyaltyVault.sol (lines 43-46)

        require(
            IERC20(royaltyAsset).transfer(splitterProxy, splitterShare) == true,
            "Failed to transfer royalty Asset to splitter"
        );

16. File: royalty-vault/contracts/RoyaltyVault.sol (lines 47-50)

        require(
            ISplitter(splitterProxy).incrementWindow(splitterShare) == true,
            "Failed to increment splitter window"
        );

17. File: royalty-vault/contracts/RoyaltyVault.sol (lines 51-57)

        require(
            IERC20(royaltyAsset).transfer(
                platformFeeRecipient,
                platformShare
            ) == true,
            "Failed to transfer royalty Asset to platform fee recipient"
        );

18. File: splits/contracts/SplitFactory.sol (lines 48-51)

    require(
      splits[_splitId] == address(0),
      'SplitFactory : Split ID already in use'
    );

19. File: splits/contracts/SplitFactory.sol (lines 81-84)

    require(
      ICoreCollection(_collectionContract).owner() == msg.sender,
      'Transaction sender is not collection owner'
    );

20. File: splits/contracts/Splitter.sol (lines 118-121)

        require(
            !isClaimed(msg.sender, window),
            "NFT has already claimed the given window"
        );

Use a more recent version of solidity

Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value

1. File: royalty-vault/contracts/RoyaltyVault.sol (line 2)

pragma solidity ^0.8.4;

2. File: royalty-vault/contracts/ProxyVault.sol (line 2)

pragma solidity ^0.8.4;

3. File: splits/contracts/SplitFactory.sol (line 2)

pragma solidity ^0.8.4;

4. File: splits/contracts/SplitProxy.sol (line 2)

pragma solidity ^0.8.4;

5. File: splits/contracts/Splitter.sol (line 2)

pragma solidity ^0.8.4;

Use a more recent version of solidity

Use a solidity version of at least 0.8.2 to get compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value

1. File: core-contracts/contracts/ERC721Payable.sol (line 2)

pragma solidity ^0.8.0;

2. File: core-contracts/contracts/CoreProxy.sol (line 2)

pragma solidity ^0.8.0;

3. File: core-contracts/contracts/ERC721Claimable.sol (line 2)

pragma solidity ^0.8.0;

4. File: core-contracts/contracts/CoreCollection.sol (line 2)

pragma solidity ^0.8.0;

5. File: core-contracts/contracts/CoreFactory.sol (line 2)

pragma solidity ^0.8.0;

Using bools for storage incurs overhead

    // Booleans are more expensive than uint256 or any type that takes up a full
    // word because each write operation emits an extra SLOAD to first read the
    // slot's contents, replace the bits taken up by the boolean, and then write
    // back. This is the compiler's defense against contract upgrades and
    // pointer aliasing, and it cannot be disabled.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27

1. File: core-contracts/contracts/ERC721Payable.sol (line 8)

  bool public isForSale;

2. File: core-contracts/contracts/CoreCollection.sol (line 20)

    bool public initialized;

Using > 0 costs more gas than != 0 when used on a uint in a require() statement

1. File: core-contracts/contracts/CoreCollection.sol (lines 52-55)

        require(
            _maxSupply > 0,
            "CoreCollection: Max supply should be greater than 0"
        );

2. File: core-contracts/contracts/CoreCollection.sol (line 146)

        require(amount > 0, "CoreCollection: Amount should be greater than 0");

3. File: royalty-vault/contracts/RoyaltyVault.sol (lines 34-37)

        require(
            balanceOfVault > 0,
            "Vault does not have enough royalty Asset to send"
        );

4. File: splits/contracts/Splitter.sol (line 164)

        require(royaltyAmount > 0, "No additional funds for window");

<array>.length should not be looked up in every loop of a for-loop

Even memory arrays incur the overhead of bit tests and bit shifts to calculate the array length

1. File: core-contracts/contracts/CoreFactory.sol (line 79)

    for (uint256 i; i < _collections.length; i++) {

2. File: splits/contracts/Splitter.sol (line 274)

        for (uint256 i = 0; i < proof.length; i++) {

It costs more gas to initialize variables to zero than to let the default of zero be applied

1. File: core-contracts/contracts/CoreCollection.sol (line 279)

        for (uint256 i = 0; i < _amount; i++) {

2. File: splits/contracts/Splitter.sol (line 49)

        uint256 amount = 0;

3. File: splits/contracts/Splitter.sol (line 50)

        for (uint256 i = 0; i < currentWindow; i++) {

4. File: splits/contracts/Splitter.sol (line 274)

        for (uint256 i = 0; i < proof.length; i++) {

State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second access of a state variable within a function. Less obvious optimizations include having local storage variables of mappings within state variable mappings or mappings within state variable structs, having local storage variables of structs within mappings, or having local caches of state variable contracts/addresses.

1. File: core-contracts/contracts/CoreCollection.sol (line 231)

        emit StartingIndexSet(startingIndex);

2. File: core-contracts/contracts/CoreCollection.sol (line 264)

        tokenId = ((startingIndex + totalSupply()) % maxSupply) + 1;

3. File: splits/contracts/SplitFactory.sol (line 161)

    delete merkleRoot;

4. File: splits/contracts/SplitFactory.sol (line 171)

    delete splitterProxy;

Using calldata instead of memory for read-only arguments in external functions saves gas

1. File: core-contracts/contracts/CoreCollection.sol (line 79)

        string memory _collectionName,

2. File: core-contracts/contracts/CoreCollection.sol (line 80)

        string memory _collectionSymbol,

3. File: core-contracts/contracts/CoreCollection.sol (line 81)

        string memory _collectionURI,

4. File: core-contracts/contracts/CoreCollection.sol (line 122)

        string memory _collectionName,

5. File: core-contracts/contracts/CoreCollection.sol (line 123)

        string memory _collectionSymbol

6. File: core-contracts/contracts/CoreFactory.sol (line 71)

    string memory _projectId,

7. File: core-contracts/contracts/CoreFactory.sol (line 72)

    Collection[] memory _collections

8. File: core-contracts/contracts/CoreFactory.sol (line 109)

    string memory _projectId,

9. File: core-contracts/contracts/CoreFactory.sol (line 110)

    Collection memory _collection

10. File: core-contracts/contracts/CoreFactory.sol (line 128)

  function getProject(string memory _projectId)

11. File: splits/contracts/SplitFactory.sol (line 79)

    string memory _splitId

12. File: splits/contracts/SplitFactory.sol (line 105)

    string memory _splitId

++i/i++ should be unchecked{++i}/unchecked{++i} when it is not possible for them to overflow, as is the case when used in for- and while-loops

1. File: core-contracts/contracts/CoreCollection.sol (line 279)

        for (uint256 i = 0; i < _amount; i++) {

2. File: core-contracts/contracts/CoreFactory.sol (line 79)

    for (uint256 i; i < _collections.length; i++) {

3. File: splits/contracts/Splitter.sol (line 50)

        for (uint256 i = 0; i < currentWindow; i++) {

4. File: splits/contracts/Splitter.sol (line 274)

        for (uint256 i = 0; i < proof.length; i++) {

++i costs less gas than ++i, especially when it's used in for-loops (--i/i-- too)

1. File: core-contracts/contracts/CoreCollection.sol (line 279)

        for (uint256 i = 0; i < _amount; i++) {

2. File: core-contracts/contracts/CoreFactory.sol (line 79)

    for (uint256 i; i < _collections.length; i++) {

3. File: splits/contracts/Splitter.sol (line 50)

        for (uint256 i = 0; i < currentWindow; i++) {

4. File: splits/contracts/Splitter.sol (line 274)

        for (uint256 i = 0; i < proof.length; i++) {

Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed

1. File: splits/contracts/Splitter.sol (line 217)

    function amountFromPercent(uint256 amount, uint32 percent)

Using private rather than public for constants, saves gas

If needed, the value can be read from the verified contract source code

1. File: splits/contracts/Splitter.sol (line 14)

    uint256 public constant PERCENTAGE_SCALE = 10e5;

2. File: splits/contracts/Splitter.sol (line 15)

    bytes4 public constant IID_IROYALTY = type(IRoyaltyVault).interfaceId;

Don't compare boolean expressions to boolean literals

if (<x> == true) => if (<x>), if (<x> == false) => if (!<x>)

1. File: royalty-vault/contracts/RoyaltyVault.sol (line 44)

            IERC20(royaltyAsset).transfer(splitterProxy, splitterShare) == true,

2. File: royalty-vault/contracts/RoyaltyVault.sol (line 48)

            ISplitter(splitterProxy).incrementWindow(splitterShare) == true,

3. File: royalty-vault/contracts/RoyaltyVault.sol (lines 52-55)

            IERC20(royaltyAsset).transfer(
                platformFeeRecipient,
                platformShare
            ) == true,

Remove unused variables

1. File: splits/contracts/Splitter.sol (line 14)

    uint256 public constant PERCENTAGE_SCALE = 10e5;

State variables only set in the constructor should be declared immutable

1. File: royalty-vault/contracts/ProxyVault.sol (line 9)

    address internal royaltyVault;

require() or revert() statements that check input arguments should be at the top of the function

1. File: splits/contracts/Splitter.sol (line 164)

        require(royaltyAmount > 0, "No additional funds for window");

private functions not called by the contract should be removed to save deployment gas

1. File: splits/contracts/Splitter.sol (lines 217-220)

    function amountFromPercent(uint256 amount, uint32 percent)
        private
        pure
        returns (uint256)

2. File: splits/contracts/Splitter.sol (lines 248-250)

    function attemptETHTransfer(address to, uint256 value)
        private
        returns (bool)

public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public.

1. File: royalty-vault/contracts/RoyaltyVault.sol (lines 95-100)

    function supportsInterface(bytes4 interfaceId)
        public
        view
        virtual
        override(IRoyaltyVault, ERC165)
        returns (bool)

2. File: royalty-vault/contracts/RoyaltyVault.sol (line 88)

    function getSplitter() public view override returns (address) {

Use custom errors rather than revert()/require() strings to save deployment gas

1. File: royalty-vault/contracts/RoyaltyVault.sol (Various lines throughout the file)
2. File: splits/contracts/SplitFactory.sol (Various lines throughout the file)
3. File: splits/contracts/Splitter.sol (Various lines throughout the file)

Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

1. File: core-contracts/contracts/CoreCollection.sol (lines 78-87)

    function initialize(
        string memory _collectionName,
        string memory _collectionSymbol,
        string memory _collectionURI,
        uint256 _maxSupply,
        uint256 _mintFee,
        address _payableToken,
        bool _isForSale,
        address _splitFactory
    ) external onlyOwner onlyValidSupply(_maxSupply) {

2. File: core-contracts/contracts/CoreCollection.sol (lines 78-87)

    function initialize(
        string memory _collectionName,
        string memory _collectionSymbol,
        string memory _collectionURI,
        uint256 _maxSupply,
        uint256 _mintFee,
        address _payableToken,
        bool _isForSale,
        address _splitFactory
    ) external onlyOwner onlyValidSupply(_maxSupply) {

3. File: core-contracts/contracts/CoreCollection.sol (lines 105-109)

    function initializeClaims(bytes32 _root)
        external
        onlyOwner
        onlyNotClaimableSet
        onlyValidRoot(_root)

4. File: core-contracts/contracts/CoreCollection.sol (lines 105-109)

    function initializeClaims(bytes32 _root)
        external
        onlyOwner
        onlyNotClaimableSet
        onlyValidRoot(_root)

5. File: core-contracts/contracts/CoreCollection.sol (lines 105-109)

    function initializeClaims(bytes32 _root)
        external
        onlyOwner
        onlyNotClaimableSet
        onlyValidRoot(_root)

6. File: core-contracts/contracts/CoreCollection.sol (lines 121-124)

    function setCollectionMeta(
        string memory _collectionName,
        string memory _collectionSymbol
    ) external onlyOwner {

7. File: core-contracts/contracts/CoreCollection.sol (lines 139-145)

    function mintToken(
        address to,
        bool isClaim,
        uint256 claimableAmount,
        uint256 amount,
        bytes32[] calldata merkleProof
    ) external onlyInitialized {

8. File: core-contracts/contracts/CoreCollection.sol (line 173)

    function withdraw() external onlyOwner {

9. File: core-contracts/contracts/CoreCollection.sol (lines 185-187)

    function setRoyaltyVault(address _royaltyVault)
        external
        onlyVaultUninitialized

10. File: core-contracts/contracts/CoreCollection.sol (line 203)

    function setHashedProof(string calldata _proof) external onlyOwner {

11. File: core-contracts/contracts/CoreFactory.sol (lines 70-73)

  function createProject(
    string memory _projectId,
    Collection[] memory _collections
  ) external onlyAvailableProject(_projectId) {

12. File: core-contracts/contracts/CoreFactory.sol (lines 108-111)

  function addCollection(
    string memory _projectId,
    Collection memory _collection
  ) external onlyProjectOwner(_projectId) returns (address) {

13. File: core-contracts/contracts/CoreFactory.sol (lines 142-145)

  function _createCollection(Collection memory _collection)
    private
    onlyAvailableCollection(_collection.id)
    returns (address)

14. File: royalty-vault/contracts/RoyaltyVault.sol (line 67)

    function setPlatformFee(uint256 _platformFee) external override onlyOwner {

15. File: royalty-vault/contracts/RoyaltyVault.sol (lines 76-79)

    function setPlatformFeeRecipient(address _platformFeeRecipient)
        external
        override
        onlyOwner

16. File: splits/contracts/SplitFactory.sol (lines 75-80)

  function createSplit(
    bytes32 _merkleRoot,
    address _splitAsset,
    address _collectionContract,
    string memory _splitId
  ) external onlyAvailableSplit(_splitId) returns (address splitProxy) {

17. File: splits/contracts/SplitFactory.sol (lines 102-106)

  function createSplit(
    bytes32 _merkleRoot,
    address _splitAsset,
    string memory _splitId
  ) external onlyAvailableSplit(_splitId) returns (address splitProxy) {

18. File: splits/contracts/SplitFactory.sol (lines 120-122)

  function setPlatformFee(address _vault, uint256 _platformFee)
    external
    onlyOwner

19. File: splits/contracts/SplitFactory.sol (lines 132-135)

  function setPlatformFeeRecipient(
    address _vault,
    address _platformFeeRecipient
  ) external onlyOwner {