Notional x Index Coop - cloudjunky's results

Inconsistent Pragma Definitions and floating pragma

Issue

There are multiple different pragma versions across the contracts in notional-wrapped-fcash/contracts. Namely;

• ^0.8.0 in contracts/lib/Constants.sol, contracts/lib/DateTime.sol, contracts/lib/Types.sol and interfaces/IERC4626.sol
• ^0.8.11 in interfaces/notional/INotionalV2.sol
• 0.8.11 in contracts/lib/EncodeDecode.sol, contracts/wfCashBase.sol and contracts/wfCashLogic.sol.
• =0.8.11 in interfaces/WETH9.sol

This is a weakness identified in SWC-103 e.g. "Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly."

Remediation

Lock pragmas to a specified compiler version e.g. pragma solidity 0.8.11 on all contracts unless they are intended to be consumed by other developers.

Require with empty error messages

Issue

There are a series of require statements that have no message so it's difficult to understand the reason why it might revert. Often there's also no natspec comments to explain the enclosing function;

Examples;

• wfCashLogic.sol lines L129 and L316.
• wfCashERC4626.sol lines L42 and L245.

Remediation

Ensure that each require statement has a clear message for why it failed. This is especially true for _safeUint88 in wfCashLogic which might revert for large values of fCash (e.g. 309485009821345068724781056).

Incomplete or inconsistent natspec

Issue

There's missing natspec documentation on most functions and important fields like @param and @return are often missing. wfCashLogic.sol has the most detailed documentation however it is still incomplete in many areas. Some examples;

• _mintInternal() has no natspec documentation.
• redeem() and redeemToAsset() have no @param and onERC1155Received has no @param or @return documentation.
• _sendTokensToReceiver() has no documentation at all despite it handling WETH and ETH transfers.

Remediation

Propert natspec documentation with @param and @return populated would significantly impreove the readability of the code base and ensure reviewers aren't guessing as to the intent of functions and variables.

Mixed approach to return statements

Issue

There's mixed use of return and option returns across the wrapped fcash codebase. Examples;

• In wfCashLogic.sol
  • onERC1155Received() and _safeUint88() use a return statement.
  • _withdrawCashToAccount() and _sendTokensToReceiver optionally return.

Combined with the natspec issue above this dramatically effects code readability and the likelihood of bugs being introduced.

Remediation

Documenting @return would really help if the style guide is to still optionally return. Either way a consistent approach would improve readability.

Deprecated ABIEncoderv2 referenced in ^0.8.0 contracts

Issue

pragma experimental ABIEncoderV2; is referenced in a contract that has a solidity pragma > 0.8.0. ABIEncoderv2 is still valid but is deprecated in Solidity 0.8.0. See the solidity docs. A reference to ABIEncoderv2 was found in contracts/wfCashLogic.sol.

Remediation

Delete the reference to pragma experimental ABIEncoderv2 in contracts/wfCashLogic.sol or if there's still a requirement to be explicit change the reference to pragma abicoder v2; similar to contracts/lib/DateTime.sol.