Joyn contest - z3s's results

Non Critical

Use of Floating Pragma:

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.

Recommended Mitigation Steps:

Lock the pragma version: use 0.8.9 instead of ^0.8.0.

Delete unused imports:

Having unused imports make confusion.

• https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreProxy.sol#L6

Code Style: non-constant should not be named in upper-case:

Non-constant (especially public) variables should not be in SNAKE_CASE, or they may be misunderstood as constants. Consider changing to camelCase. https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L27 it's value can be changed in: https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L209

Redundant == true in require:

transfer return boolean and require expect a boolean. so these == true can be deleted: 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#L55

Low Risks

Use better reentrancy attack prevention method than gas limit:

https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/Splitter.sol#L255 As mentioned in comments this can lead to limited reentrancy attack. use openzeppelin reentrancyguard here. this internal function it's not used yet in other parts of code. but use a reentrancy attack prevention method when you going to use it.

Gas Optimizations

Saving some SLOADs in sendToSplitter():

https://github.com/code-423n4/2022-03-joyn/blob/main/royalty-vault/contracts/RoyaltyVault.sol#L31 cache splitterProxy, royaltyAsset, platformFeeRecipient in sendToSplitter(). each direct read of state vars cost 100 gas.

Use calldata instead of memory in createProject():

https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreFactory.sol#L72

For external function's dynamic params, calldata is the cheapest location to use.

Recommended Mitigation Steps:

Change params memory to calldata

Use Custom Errors to save Gas:

Custom errors from Solidity 0.8.4 are cheaper than require messages. https://blog.soliditylang.org/2021/04/21/custom-errors/

++i use less gas than i++:

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#L79 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

Redundant == true in require:

transfer return boolean and require expect a boolean. so removing these == true can save some gas: 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#L55