QA Report

[code423n4]

[N1] Unused imports

The following source units are imported but not referenced in the contract:

https://github.com/code-423n4/2022-02-nested/blob/879bae87b1987d6810f25c1082e5bf664390ae7f/contracts/operators/Flat/FlatOperator.sol#L4-L4

import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

https://github.com/code-423n4/2022-02-nested/blob/879bae87b1987d6810f25c1082e5bf664390ae7f/contracts/operators/Flat/IFlatOperator.sol#L4-L4

import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

Recommendation

Check all imports and remove all unused/unreferenced and unnecessary imports.

[N2] Using public to generate the getter function can make the code simpler and cleaner

https://github.com/code-423n4/2022-02-nested/blob/879bae87b1987d6810f25c1082e5bf664390ae7f/contracts/operators/ZeroEx/ZeroExStorage.sol#L7-L19

contract ZeroExStorage is Ownable {
    address private _swapTarget;

    /// @notice Returns the address of 0x swaptarget
    function swapTarget() external view returns (address) {
        return _swapTarget;
    }

    /// @notice Update the address of 0x swaptarget
    function updatesSwapTarget(address swapTargetValue) external onlyOwner {
        _swapTarget = swapTargetValue;
    }
}

Can be changed to:

contract ZeroExStorage is Ownable {
    address public swapTarget;

    /// @notice Update the address of 0x swaptarget
    function updatesSwapTarget(address swapTargetValue) external onlyOwner {
        swapTarget = swapTargetValue;
    }
}

[N3] Inconsistent use of _msgSender()

Direct use of msg.sender vs internal call of _msgSender().

https://github.com/code-423n4/2022-02-nested/blob/879bae87b1987d6810f25c1082e5bf664390ae7f/contracts/operators/ZeroEx/ZeroExOperator.sol#L17-L17

ZeroExStorage(operatorStorage).transferOwnership(msg.sender);

https://github.com/code-423n4/2022-02-nested/blob/879bae87b1987d6810f25c1082e5bf664390ae7f/contracts/NestedReserve.sol#L30-L30

_token.safeTransfer(msg.sender, _amount);

https://github.com/code-423n4/2022-02-nested/blob/879bae87b1987d6810f25c1082e5bf664390ae7f/contracts/abstracts/OwnableFactoryHandler.sol#L21-L21

require(supportedFactories[msg.sender], "OFH: FORBIDDEN");

https://github.com/code-423n4/2022-02-nested/blob/879bae87b1987d6810f25c1082e5bf664390ae7f/contracts/FeeSplitter.sol#L103-L103

require(msg.sender == weth, "FS: ETH_SENDER_NOT_WETH");

https://github.com/code-423n4/2022-02-nested/blob/879bae87b1987d6810f25c1082e5bf664390ae7f/contracts/FeeSplitter.sol#L166-L168

amount = _releaseToken(_msgSender(), _tokens[i]);
_tokens[i].safeTransfer(_msgSender(), amount);
emit PaymentReleased(_msgSender(), address(_tokens[i]), amount);

https://github.com/code-423n4/2022-02-nested/blob/879bae87b1987d6810f25c1082e5bf664390ae7f/contracts/FeeSplitter.sol#L199-L199

_token.safeTransferFrom(_msgSender(), address(this), _amount);

Recommendation

Consider replacing _msgSender() with msg.sender for consistency.

[maximebrugel]

"Unused imports" (Confirmed)

"Using public to generate the getter function can make the code simpler and cleaner" (Confirmed)

"Inconsistent use of _msgSender()" (Disputed)

The one with msg.sender are used when meta transactions are not supported (sender is WETH, Nested Factory,...)

[harleythedogC4]

My personal judgements:

1. "[N1] Unused imports". Valid and non-critical.
2. "[N2] Using public to generate the getter function can make the code simpler and cleaner". Valid and non-critical.
3. "[N3] Inconsistent use of _msgSender()". As sponsor describes. Invalid.

[harleythedogC4]

Now, here is the methodology I used for calculating a score for each QA report. I first assigned each submission to be either non-critical (1 point), very-low-critical (5 points) or low-critical (10 points), depending on how severe/useful the issue is. The score of a QA report is the sum of these points, divided by the maximum number of points achieved by a QA report. This maximum number was 26 points, achieved by https://github.com/code-423n4/2022-02-nested-findings/issues/66.

The number of points achieved by this report is 2 points. Thus the final score of this QA report is (2/26)*100 = 8.