QA Report

[code423n4]

1. Title : using pragma abicoder V2

Since wfCashLogic.sol was using pragma experimental ABIEncoderV2 and used pragma ^0.7.6, it because the ABI coder v2 is not considered experimental anymore, it can be selected via pragma abicoder v2 instead since Solidity 0.7.4.

Tools

Manual Review

Recommended Mitigation

you can change it into pragma abicoder v2;

2. The way to use ABIEncoderV2

The standart used for pragma ABIEncoder for pragma solidity 0.6.10;

pragma experimental ABIEncoderV2;

Since it was good to use for usually for good readibility and code as well.

Tool Used

Manual Review

3. Title : unclear comment becoming confusing

1.) File : wfCashLogic.sol Line.98

Since this was only used in function _mintInternal() and unclear what was operatorAck does to be used since cause it was one comment, that means operator Acknowledge (operatorAck). This was uncleared as an information. what was operatorAck does. if would be not do an operatorAck, comment can be deleted instead.

2.) File : NotionalTradeModule.sol

416 : * @dev Alo adjust the components / position of the set token accordingly

455 : * @dev Alo adjust the components / position of the set token accordingly

This Alo was confusing since didn't know what it is stand for. It is typo? or it can be cleared as well.

Tool Used

Manual Review

4. Title : Avoid floating pragmas: the version should be locked

The pragma declared at wfCashERC4626.sol was used ^0.8.0. As the compiler can be use as 0.8.11 and consider locking at this version the same as another.

Tool Used

Manual Review

5. Title : Related data should be grouped in a struct

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L111-L118

The following maps , it can be grouped in structs.

    mapping(ISetToken => bool) public redeemToUnderlying;
    mapping(ISetToken => bool) public allowedSetTokens;
    bool public anySetAllowed;

Tool Used

Manual Review

6. Title : Simplify return

returns a value to the main program when exiting a function. You can do further operations on the returned value. This implementation below can be used to simplify code and good readibility.

1.) File : contracts/protocol/modules/v1/NotionalTradeModule.sol Line 172

        return _mintFCashPosition(_setToken, wrappedfCash, IERC20(_sendToken), _mintAmount, _maxSendAmount);

into :

        return _mintFCashPosition;

2.) File : contracts/protocol/modules/v1/NotionalTradeModule.sol Line 201

        return _redeemFCashPosition(_setToken, wrappedfCash, IERC20(_receiveToken), _redeemAmount, _minReceiveAmount);

into :

        return _redeemFCashPosition;

7. Title : Typo Comment

1.) File : contracts/protocol/modules/v1/NotionalTradeModule.sol Line 215

MNGER change to MANAGER

8. Title : Require Statement missing Reason String

1.) File : contracts/wfCashERC4626.sol Line. 42

        require(pvExternal >= 0);

2.) File : contracts/wfCashERC4626.sol Line. 245

        require(int256(type(int88).min) <= y);

3.) File : contracts/wfCashLogic.sol Line. 316

        require(x <= uint256(type(uint88).max));

9. Title : Consider make the constracts pausable

There are many external risks so the suggestion was it should be consider making the contracts pausable, so if in the case of an unexpected event, the admin can pause transfers.

Tool Used

Manual Review

POC

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/Pausable.sol

Recommended Mitigation Steps

Consider making contracts Pausable