Notional x Index Coop - Meera's results

Judge has assessed an item in Issue #150 as Medium risk. The relevant finding follows:

Unsafe casting may overflow

SafeMath and Solidity 0.8.* handles overflows for basic math operations but not for casting. Consider using OpenZeppelin's SafeCast library to prevent unexpected overflows when casting from uint256 here:

index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:
  526:             uint88(_fCashAmount),

notional-wrapped-fcash/contracts/wfCashBase.sol:
  118:         return uint8(marketIndex);

Notice that a solution has been coded here:

File: wfCashLogic.sol
315:     function _safeUint88(uint256 x) internal pure returns (uint88) {
316:         require(x <= uint256(type(uint88).max));
317:         return uint88(x);
318:     }

Unsafe casting may overflow

SafeMath and Solidity 0.8.* handles overflows for basic math operations but not for casting. Consider using OpenZeppelin's SafeCast library to prevent unexpected overflows when casting from uint256 here:

index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:
  526:             uint88(_fCashAmount),

notional-wrapped-fcash/contracts/wfCashBase.sol:
  118:         return uint8(marketIndex);

Notice that a solution has been coded here:

File: wfCashLogic.sol
315:     function _safeUint88(uint256 x) internal pure returns (uint88) {
316:         require(x <= uint256(type(uint88).max));
317:         return uint88(x);
318:     }

A similar solution would be enough.

Missing address(0) checks

In the constructors, the solution never checks for address(0) when setting the value of immutable variables. I suggest adding those checks.

List of immutable variables:

index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:
  121:     IWrappedfCashFactory public immutable wrappedfCashFactory;
  122:     IERC20 public immutable weth;

notional-wrapped-fcash/contracts/wfCashBase.sol:
  20:     INotionalV2 public immutable NotionalV2;
  21:     WETH9 public immutable WETH;

notional-wrapped-fcash/contracts/proxy/WrappedfCashFactory.sol:
  11:     address public immutable BEACON;

Unbounded loop on array can lead to DoS

As these arrays can grow quite large (only push operations, no pop), the transaction's gas cost could exceed the block gas limit and make it impossible to call the impacted functions at all.

index-coop-notional-trade-module/contracts/protocol/SetToken.sol:
  176:         for (uint256 i = 0; i < _modules.length; i++) {
  181:         for (uint256 j = 0; j < _components.length; j++) {
  220:         components.push(_component);
  252:         componentPositions[_component].externalPositionModules.push(_positionModule);
  411:         modules.push(msg.sender);
  485:         for (uint256 i = 0; i < components.length; i++) {
  502:             for (uint256 j = 0; j < externalModules.length; j++) {
  527:         for (uint256 i = 0; i < externalModules.length; i++) {
  604:         for (uint256 i = 0; i < components.length; i++) {
  614:             for (uint256 j = 0; j < externalModules.length; j++) {
  636:         for (uint256 i = 0; i < components.length; i++) {

index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:
  278:         issuanceSettings[_setToken].moduleIssuanceHooks.push(msg.sender);
  656:         for (uint256 i = 0; i < issuanceHooks.length; i++) {
  666:         for (uint256 i = 0; i < issuanceHooks.length; i++) {

Consider introducing a reasonable upper limit based on block gas limits and adding a method to remove elements in the array.

Lack of event emission after critical initialize() functions

To record the init parameters for off-chain monitoring and transparency reasons, please consider emitting an event after the initialize() functions:

index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:
  305:     function initialize(

index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:
  219:     function initialize(

notional-wrapped-fcash/contracts/wfCashBase.sol:
  35:     function initialize(uint16 currencyId, uint40 maturity) external override initializer {

Add a timelock to critical functions

It is a good practice to give time for users to react and adjust to critical changes. A timelock provides more guarantees and reduces the level of trust required, thus decreasing risk for users. It also indicates that the project is legitimate (less risk of a malicious owner making a sandwich attack on a user).

Consider adding a timelock to:

index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:
  234:     function updateIssueFee(
  255:     function updateRedeemFee(

Use a more recent version of solidity to get string.concat() (0.8.12 vs 0.8.11)

Use a solidity version of at least 0.8.12 to get string.concat() to be used instead of abi.encodePacked(<str>,<str>)

notional-wrapped-fcash/contracts/wfCashBase.sol:
   2: pragma solidity 0.8.11;
  59:             string(abi.encodePacked("Wrapped f", _symbol, " @ ", _maturity)),
  61:             string(abi.encodePacked("wf", _symbol, ":", _maturity)),

notional-wrapped-fcash/contracts/proxy/WrappedfCashFactory.sol:
   2: pragma solidity 0.8.11;
  23:         return abi.encodePacked(type(nBeaconProxy).creationCode, abi.encode(BEACON, initCallData));

Several functions are declaring named returns but then are using return statements. I suggest choosing only one for readability reasons. Details below

While not consuming more gas with the Optimizer enabled: using both named returns and a return statement isn't necessary. Removing one of those can improve code clarity:

index-coop-notional-trade-module/contracts/protocol/SetToken.sol:
  205:         returns (bytes memory _returnValue) //@audit NC: unused named return
  211:         return _returnValue; //@audit NC: unused named return

notional-wrapped-fcash/contracts/wfCashBase.sol:
  124:     function getUnderlyingToken() public view override returns (IERC20 underlyingToken, int256 underlyingPrecision) { //@audit NC: unused named return
  129:             return (IERC20(asset.tokenAddress), asset.decimals); //@audit NC: unused named return
  131:             return (IERC20(underlying.tokenAddress), underlying.decimals); //@audit NC: unused named return

  137:     function getAssetToken() public view override returns (IERC20 assetToken, int256 underlyingPrecision, TokenType tokenType) { //@audit NC: unused named return
  139:         return (IERC20(asset.tokenAddress), asset.decimals, asset.tokenType); //@audit NC: unused named return

notional-wrapped-fcash/contracts/wfCashERC4626.sol:
  52:     function convertToShares(uint256 assets) public view override returns (uint256 shares) { //@audit NC: unused named return
  57:             return (assets * uint256(Constants.INTERNAL_TOKEN_PRECISION)) / unitfCashValue; //@audit NC: unused named return
  60:         return (assets * totalSupply()) / totalAssets(); //@audit NC: unused named return
  
  64:     function convertToAssets(uint256 shares) public view override returns (uint256 assets) { //@audit NC: unused named return
  68:             return _getPresentValue(shares); //@audit NC: unused named return
  71:         return (shares * totalAssets()) / supply; //@audit NC: unused named return

The pragmas used are not the same everywhere

index-coop-notional-trade-module/contracts/protocol/SetToken.sol:
  19: pragma solidity 0.6.10;

index-coop-notional-trade-module/contracts/protocol/integration/wrap/notional/WrappedfCash.sol:
  2: pragma solidity 0.8.11;

index-coop-notional-trade-module/contracts/protocol/integration/wrap/notional/WrappedfCashFactory.sol:
  2: pragma solidity 0.8.11;

index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:
  19: pragma solidity 0.6.10;

index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:
  19: pragma solidity 0.6.10;

notional-wrapped-fcash/contracts/wfCashBase.sol:
  2: pragma solidity 0.8.11;

notional-wrapped-fcash/contracts/wfCashERC4626.sol:
  2: pragma solidity ^0.8.0;

notional-wrapped-fcash/contracts/wfCashLogic.sol:
  2: pragma solidity 0.8.11;

notional-wrapped-fcash/contracts/proxy/WrappedfCashFactory.sol:
  2: pragma solidity 0.8.11;

notional-wrapped-fcash/interfaces/notional/INotionalV2.sol:
  2: pragma solidity ^0.8.11;

notional-wrapped-fcash/interfaces/notional/IWrappedfCash.sol:
  2: pragma solidity ^0.8.0;

Avoid floating pragmas: the version should be locked

notional-wrapped-fcash/contracts/wfCashERC4626.sol:
  2: pragma solidity ^0.8.0;

notional-wrapped-fcash/interfaces/notional/INotionalV2.sol:
  2: pragma solidity ^0.8.11;

notional-wrapped-fcash/interfaces/notional/IWrappedfCash.sol:
  2: pragma solidity ^0.8.0;

Table of Contents:

• An array's length should be cached to save gas in for-loops
• ++i costs less gas compared to i++ or i += 1 (same for --i vs i-- or i -= 1)
• abi.encode() is less efficient than abi.encodePacked()
• No need to explicitly initialize variables with default values
• Upgrade pragma to at least 0.8.4
• Reduce the size of error messages (Long revert Strings)
• Use Custom Errors instead of Revert Strings to save Gas
• Functions guaranteed to revert when called by normal users can be marked payable

An array's length should be cached to save gas in for-loops

Reading array length at each iteration of the loop consumes more gas than necessary.

In the best case scenario (length read on a memory variable), caching the array length in the stack saves around 3 gas per iteration. In the worst case scenario (external calls at each iteration), the amount of gas wasted can be massive.

Here, I suggest storing the array's length in a variable before the for-loop, and use this new variable instead:

index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:471:        for (uint256 i = 0; i < components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:508:        for (uint256 i = 0; i < components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:515:                for (uint256 j = 0; j < externalPositions.length; j++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:550:        for (uint256 i = 0; i < _components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:590:        for (uint256 i = 0; i < _components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:656:        for (uint256 i = 0; i < issuanceHooks.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:666:        for (uint256 i = 0; i < issuanceHooks.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:238:        for(uint256 i = 0; i < modules.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:254:        for(uint256 i = 0; i < modules.length; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:176:        for (uint256 i = 0; i < _modules.length; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:181:        for (uint256 j = 0; j < _components.length; j++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:485:        for (uint256 i = 0; i < components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:502:            for (uint256 j = 0; j < externalModules.length; j++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:527:        for (uint256 i = 0; i < externalModules.length; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:604:        for (uint256 i = 0; i < components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:614:            for (uint256 j = 0; j < externalModules.length; j++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:636:        for (uint256 i = 0; i < components.length; i++) {

++i costs less gas compared to i++ or i += 1 (same for --i vs i-- or i -= 1)

Pre-increments and pre-decrements are cheaper.

For a uint256 i variable, the following is true with the Optimizer enabled at 10k:

Increment:

• i += 1 is the most expensive form
• i++ costs 6 gas less than i += 1
• ++i costs 5 gas less than i++ (11 gas less than i += 1)

Decrement:

• i -= 1 is the most expensive form
• i-- costs 11 gas less than i -= 1
• --i costs 5 gas less than i-- (16 gas less than i -= 1)

Note that post-increments (or post-decrements) return the old value before incrementing or decrementing, hence the name post-increment:

uint i = 1;  
uint j = 2;
require(j == i++, "This will be false as i is incremented after the comparison");

However, pre-increments (or pre-decrements) return the new value:

uint i = 1;  
uint j = 2;
require(j == ++i, "This will be true as i is incremented before the comparison");

In the pre-increment case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2.

Affected code:

index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:471:        for (uint256 i = 0; i < components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:508:        for (uint256 i = 0; i < components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:515:                for (uint256 j = 0; j < externalPositions.length; j++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:550:        for (uint256 i = 0; i < _components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:590:        for (uint256 i = 0; i < _components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:656:        for (uint256 i = 0; i < issuanceHooks.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:666:        for (uint256 i = 0; i < issuanceHooks.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:689:            for (uint256 i = 0; i < modulesLength; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:693:            for (uint256 i = 0; i < modulesLength; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:238:        for(uint256 i = 0; i < modules.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:254:        for(uint256 i = 0; i < modules.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:393:        for(uint256 i = 0; i < positionsLength; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:605:        for(uint256 i = 0; i < positionsLength; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:610:                    numFCashPositions++;
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:618:        for(uint256 i = 0; i < positionsLength; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:623:                    j++;
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:176:        for (uint256 i = 0; i < _modules.length; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:181:        for (uint256 j = 0; j < _components.length; j++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:485:        for (uint256 i = 0; i < components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:498:                positionCount++;
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:502:            for (uint256 j = 0; j < externalModules.length; j++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:513:                positionCount++;
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:527:        for (uint256 i = 0; i < externalModules.length; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:604:        for (uint256 i = 0; i < components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:614:            for (uint256 j = 0; j < externalModules.length; j++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:636:        for (uint256 i = 0; i < components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:641:                positionCount++;

Consider using pre-increments and pre-decrements where they are relevant (meaning: not where post-increments/decrements logic are relevant).

abi.encode() is less efficient than abi.encodePacked()

Changing abi.encode function to abi.encodePacked can save gas since the abi.encode function pads extra null bytes at the end of the call data, which is unnecessary. Also, in general, abi.encodePacked is more gas-efficient (see Solidity-Encode-Gas-Comparison).

Consider using abi.encodePacked() here:

notional-wrapped-fcash/contracts/wfCashERC4626.sol:230:        bytes memory userData = abi.encode(
notional-wrapped-fcash/contracts/wfCashLogic.sol:159:        bytes memory data = abi.encode(opts);

No need to explicitly initialize variables with default values

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address...). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

As an example: for (uint256 i = 0; i < numIterations; ++i) { should be replaced with for (uint256 i; i < numIterations; ++i) {

Affected code:

index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:471:        for (uint256 i = 0; i < components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:508:        for (uint256 i = 0; i < components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:511:            int256 cumulativeDebt = 0;
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:515:                for (uint256 j = 0; j < externalPositions.length; j++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:550:        for (uint256 i = 0; i < _components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:590:        for (uint256 i = 0; i < _components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:656:        for (uint256 i = 0; i < issuanceHooks.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:666:        for (uint256 i = 0; i < issuanceHooks.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:689:            for (uint256 i = 0; i < modulesLength; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:693:            for (uint256 i = 0; i < modulesLength; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:238:        for(uint256 i = 0; i < modules.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:254:        for(uint256 i = 0; i < modules.length; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:393:        for(uint256 i = 0; i < positionsLength; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:519:        uint32 minImpliedRate = 0;
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:605:        for(uint256 i = 0; i < positionsLength; i++) {
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:618:        for(uint256 i = 0; i < positionsLength; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:176:        for (uint256 i = 0; i < _modules.length; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:181:        for (uint256 j = 0; j < _components.length; j++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:483:        uint256 positionCount = 0;
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:485:        for (uint256 i = 0; i < components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:502:            for (uint256 j = 0; j < externalModules.length; j++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:527:        for (uint256 i = 0; i < externalModules.length; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:604:        for (uint256 i = 0; i < components.length; i++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:614:            for (uint256 j = 0; j < externalModules.length; j++) {
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:636:        for (uint256 i = 0; i < components.length; i++) {

I suggest removing explicit initializations for default values.

Upgrade pragma to at least 0.8.4

Using newer compiler versions and the optimizer give gas optimizations. Also, additional safety checks are available for free.

The advantages here are:

• Low level inliner (>= 0.8.2): Cheaper runtime gas (especially relevant when the contract has small functions).
• Optimizer improvements in packed structs (>= 0.8.3)
• Custom errors (>= 0.8.4): cheaper deployment cost and runtime cost. Note: the runtime cost is only relevant when the revert condition is met. In short, replace revert strings by custom errors.

Consider upgrading pragma to at least 0.8.4:

index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:19:pragma solidity 0.6.10;
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:19:pragma solidity 0.6.10;
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:19:pragma solidity 0.6.10;

Reduce the size of error messages (Long revert Strings)

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Revert strings > 32 bytes:

index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:220:        require(_newFeeRecipient != address(0), "Fee Recipient must be non-zero address.");
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:317:        require(_managerIssueFee <= _maxManagerFee, "Issue fee can't exceed maximum fee");
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:318:        require(_managerRedeemFee <= _maxManagerFee, "Redeem fee can't exceed maximum fee");
index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol:336:        require(issuanceSettings[ISetToken(msg.sender)].moduleIssuanceHooks.length == 0, "Registered modules must be removed.");
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:169:        require(_setToken.isComponent(address(_sendToken)), "Send token must be an index component");
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:199:        require(_setToken.isComponent(address(wrappedfCash)), "FCash to redeem must be an index component");
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:378:        require(wrappedfCashAddress.isContract(), "WrappedfCash not deployed for given parameters");
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:573:            require(_paymentToken == assetToken, "Token is neither asset nor underlying token");
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:565:            revert("Real to Virtual unit conversion invalid");
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:570:            revert("Virtual to Real unit conversion invalid");
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:676:            "Module must be enabled on controller"
index-coop-notional-trade-module/contracts/protocol/SetToken.sol:686:            require(msg.sender == locker, "When locked, only the locker can call");

I suggest shortening the revert strings to fit in 32 bytes.

Use Custom Errors instead of Revert Strings to save Gas

Solidity 0.8.4 introduced custom errors. They are more gas efficient than revert strings, when it comes to deploy cost as well as runtime cost when the revert condition is met. Use custom errors instead of revert strings for gas savings.

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met)

Source: https://blog.soliditylang.org/2021/04/21/custom-errors/:

Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).

I suggest replacing all revert strings with custom errors in the following files:

index-coop-notional-trade-module/contracts/protocol/integration/wrap/notional/WrappedfCash.sol:
  2: pragma solidity 0.8.11;

index-coop-notional-trade-module/contracts/protocol/integration/wrap/notional/WrappedfCashFactory.sol:
  2: pragma solidity 0.8.11;

notional-wrapped-fcash/contracts/wfCashBase.sol:
  2: pragma solidity 0.8.11;

notional-wrapped-fcash/contracts/wfCashERC4626.sol:
  2: pragma solidity ^0.8.0;

notional-wrapped-fcash/contracts/wfCashLogic.sol:
  2: pragma solidity 0.8.11;

notional-wrapped-fcash/contracts/proxy/WrappedfCashFactory.sol:
  2: pragma solidity 0.8.11;

notional-wrapped-fcash/interfaces/notional/INotionalV2.sol:
  2: pragma solidity ^0.8.11;

notional-wrapped-fcash/interfaces/notional/IWrappedfCash.sol:
  2: pragma solidity ^0.8.0;

Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:279:    function updateAllowedSetToken(ISetToken _setToken, bool _status) external onlyOwner {
index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol:289:    function updateAnySetAllowed(bool _anySetAllowed) external onlyOwner {