QA Report

[code423n4]

Issue 1 (Low) - All function inputs should verify address != address(0)

https://github.com/code-423n4/2022-03-joyn/blob/c9297ccd925ebb2c44dbc6eaa3effd8db5d2368a/core-contracts/contracts/CoreCollection.sol#L193
https://github.com/code-423n4/2022-03-joyn/blob/c9297ccd925ebb2c44dbc6eaa3effd8db5d2368a/splits/contracts/SplitFactory.sol#L60-L61
https://github.com/code-423n4/2022-03-joyn/blob/c9297ccd925ebb2c44dbc6eaa3effd8db5d2368a/royalty-vault/contracts/RoyaltyVault.sol#L76

There are several input addresses that should be verified != address(0). Most notably, the royaltyVault and platformFeeRecipient.

Issue 2 (Low) - platformFeeRecipient must be trusted

https://github.com/code-423n4/2022-03-joyn/blob/c9297ccd925ebb2c44dbc6eaa3effd8db5d2368a/royalty-vault/contracts/RoyaltyVault.sol#L51-L57

In the event that royaltyAsset gives control on transfer such as with an ERC777, a malicious platformFeeRecipient will have the power to stop all executions of sendToSplitter()

Issue 3 (Low) - Function definition doesn't match purpose

https://github.com/code-423n4/2022-03-joyn/blob/c9297ccd925ebb2c44dbc6eaa3effd8db5d2368a/splits/contracts/Splitter.sol#L231-L240

The transferSplitAsset() function contains comments about sending ETH, and the event that is emitted is called TransferETH despite only ERC20 transfers.

Issue 4 (Non-critical) - Private attemptETHTransfer() never used

https://github.com/code-423n4/2022-03-joyn/blob/c9297ccd925ebb2c44dbc6eaa3effd8db5d2368a/splits/contracts/Splitter.sol#L248

Since attemptETHTransfer() is private and not used in the contract, it can safely be removed.

Issue 5 (non-critical) - Internal functions should start with underscore

Example:
https://github.com/code-423n4/2022-03-joyn/blob/c9297ccd925ebb2c44dbc6eaa3effd8db5d2368a/core-contracts/contracts/MultiSigWallet.sol#L284

Code style best practice.

Issue 6 (non-critical) - Unnecessary bool in TransferETH event

https://github.com/code-423n4/2022-03-joyn/blob/c9297ccd925ebb2c44dbc6eaa3effd8db5d2368a/splits/contracts/Splitter.sol#L237-L239

The function reverts if transfer fails, so bool in event will always be true.

[sofianeOuafir]

high quality report