QA Report

[code423n4]

Non-Critical

Clarity

/// @dev Storage slot for fCash id. Read only and set on initialization @audit Read-only

proposed change:

/// @dev Storage slot for fCash id. Read-only and set on initialization

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashBase.sol#L23

Typos

/// @notice Returns the components of the fCash idd @audit idd?

proposed change:

/// @notice Returns the components of the fCash ID

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashBase.sol#L97

// but might be good safe guards @audit safeguards

proposed change:

// but might be good safeguards 

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashLogic.sol#L126

* @dev MANGER ONLY: Initialize given SetToken with initial list of registered fCash positions @audit MANAGER ONLY?

proposed change:

* @dev MANAGER ONLY: Initialize given SetToken with initial list of registered fCash positions @audit MANAGER ONLY?

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

// Mapping for a set token, wether or not to redeem to underlying upon reaching maturity @audit whether

proposed change:

// Mapping for a set token, whether or not to redeem to underlying upon reaching maturity

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

Grammatical Errors

// is more gas efficient (does not require and additional redeem call to asset tokens). If using cETH @audit and additional? doesn't seem right, maybe an additional

proposed change:

// is more gas efficient (does not require an additional redeem call to asset tokens). If using cETH 

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashLogic.sol#L66

* @dev Checks if a given address is an fCash position that was deployed from the factory @audit a fCash

proposed change:

* @dev Checks if a given address is a fCash position that was deployed from the factory @audit a fCash

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

Natspec Errors

The constructor lacks a param natspec for _weth

/**
     * @dev Instantiate addresses
     * @param _controller                       Address of controller contract
     * @param _wrappedfCashFactory              Address of fCash wrapper factory used to check and deploy wrappers
     */
    constructor(
        IController _controller,
        IWrappedfCashFactory _wrappedfCashFactory,
        IERC20 _weth

    )

proposed change:

/**
     * @dev Instantiate addresses
     * @param _controller                       Address of controller contract
     * @param _wrappedfCashFactory              Address of fCash wrapper factory used to check and deploy wrappers
       @param _weth                             Address of WETH contract
     */
    constructor(
        IController _controller,
        IWrappedfCashFactory _wrappedfCashFactory,
        IERC20 _weth

    )

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

Consistency

It is recommended to change the function name from getDecodedID to getDecodedId to maintain a level of naming consistency between all the Id functions

function getDecodedID() public view override returns (uint16 currencyId, uint40 maturity) {

proposed change:

function getDecodedId() public view override returns (uint16 currencyId, uint40 maturity) {

linhttps://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashBase.sol#L98

Low-Risk

safeApprove is deprecated in favor of safeIncreaseAllowance

According to OZ documentation safeApproved is deprecated in favor of safeIncreaseAllowance: openzeppelin-contracts/SafeERC20.sol at bfff03c0d2a59bcd8e2ead1da9aed9edf0080d05 · OpenZeppelin/openzeppelin-contracts · GitHub

assetToken.safeApprove(address(NotionalV2), type(uint256).max);
        if (
            address(assetToken) != address(underlyingToken) &&
            address(underlyingToken) != Constants.ETH_ADDRESS
        ) {
            underlyingToken.safeApprove(address(NotionalV2), type(uint256).max);
        }

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashBase.sol#L68-L74