Joyn contest - hake's results

LOW

Low#1: Use safeTransferFrom instead of transferFrom or implement bool check.

CoreCollection.withdraw Failure to check if transferFrom might result in silent failures of transfers. I recommend using something like OpenZeppelin’s safeTransfer/safeTransferFrom or introducing a require() statement that checks the return value of token transfers.

Low+Gas#2: Initializing variable to default value is unnecessary and using a prefix(++i) is more efficient than using a postfix(i++)

CoreCollection.batchmint

Suggest changing from this:

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

To this:

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

Note: There is a duplicate of the gas part in the gas report, but I thought it would be more neat to show both changes together.

Low#3: No zero address checks in constructor.

CoreFactory.constructor

A new contract will have to be launched if _collection or _splitFactory are set to zero by mistake as there are no other ways to change them.

GAS

Gas#1: Using a prefix(++i) is more efficient than using a postfix(i++)

CoreCollection.batchmint

Suggest changing from this:

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

To this:

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

Gas#2: Cachingcached _collections.length can save gas

CoreFactory.createProject

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

    for (uint256 i; i < _collections.length; i++) {
      Collection memory _collection = _collections[i];
      address coreCollection = _createCollection(_collection);

_collections.length is used at least twice in createProject. By caching it, less gas will be used.

Example:

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

    for (uint256 i; i < _collections.length; i++) {
      Collection memory _collection = _collections[i];
      address coreCollection = _createCollection(_collection);