code-423n4 / 2022-06-notional-coop-findings

1 stars 1 forks source link

QA Report #177

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

[L-01] Immutable addresses should 0-Check

I recommend adding check of 0-address for immutable addresses. Not doing so might lead to non-functional contract when it is updated to 0-address accidentally.

Issue found at

NotionalTradeModule.sol
140:        wrappedfCashFactory = _wrappedfCashFactory;
141:        weth = _weth;
wfCashBase.sol
30:        NotionalV2 = _notional;
31:        WETH = _weth;
WrappedfCashFactory.sol
18:        BEACON = _beacon;

[N-01] Event is missing indexed fields

Each event should have 3 indexed fields if there are 3 or more fields.

Issue found at

WrappedfCashFactory.sol
15:    event WrapperDeployed(uint16 currencyId, uint40 maturity, address wrapper);

[N-02] Unnecessary use of named returns

Several function adds return statement even thought named returns variable are used. Remove unnecessary named returns variable to improve code readability. Also keeping the use of named returns or return statement consistent through out the whole project if possible is recommended.

Issue found at

  1. NotionalTradeModule.sol (remove returns variable positions)
    355:    function getFCashPositions(ISetToken _setToken)
    356:    external
    357:    view
    358:    returns(address[] memory positions)
    359:    {
    360:        return _getFCashPositions(_setToken);
    361:    }
  2. wfCashERC4626.sol (remove returns variable shares)
    52:    function convertToShares(uint256 assets) public view override returns (uint256 shares) {
    57:            return (assets * uint256(Constants.INTERNAL_TOKEN_PRECISION)) / unitfCashValue;
    60:        return (assets * totalSupply()) / totalAssets();
  3. wfCashERC4626.sol (remove returns variable assets)
    64:    function convertToAssets(uint256 shares) public view override returns (uint256 assets) {
    68:            return _getPresentValue(shares);
    71:        return (shares * totalAssets()) / supply;
  4. wfCashBase.sol (remove returns variable "underlyingToken" and "underlyingPrecision")
    124:    function getUnderlyingToken() public view override returns (IERC20 underlyingToken, int256 underlyingPrecision) {
    129:            return (IERC20(asset.tokenAddress), asset.decimals);
    130:        } else {
    131:            return (IERC20(underlying.tokenAddress), underlying.decimals);
  5. wfCashBase.sol (remove returns variable "assetToken", "underlyingPrecision" and "tokenType")
    137:    function getAssetToken() public view override returns (IERC20 assetToken, int256 underlyingPrecision, TokenType tokenType) {
    139:        return (IERC20(asset.tokenAddress), asset.decimals, asset.tokenType);