QA Report

[code423n4]

QA

Natspec is incomplete

File: Creator.sol line 39

Missing @return

  /// @param d Decimals of the new market zcToken
  function create (
    uint8 p,
    address u,
    uint256 m,
    address c,
    address sw,
    string memory n,
    string memory s,
    uint8 d
  ) external authorized(marketPlace) returns (address, address) {

File: Creator.sol line 46-50

Missing @return

  /// @param a Address of a new admin
  function setAdmin(address a) external authorized(admin) returns (bool) {
    admin = a;
    return true;
  }

Lock pragmas to specific compiler version

Contracts should be deployed with the same compiler version and flags that they have been tested the most with. Locking the pragma helps ensure that contracts do not accidentally get deployed using, for example, the latest compiler which may have higher risks of undiscovered bugs. Contracts may also be deployed by others and the pragma indicates the compiler version intended by the original authors.

File: ZcToken.sol line 2

pragma solidity ^0.8.4;

Unused named return

Using both named returns and a return statement isn’t necessary in a function.To improve code quality, consider using only one of those.

File: MarketPlace.sol line 148-168

  function authRedeem(uint8 p, address u, uint256 m, address f, address t, uint256 a) public authorized(markets[p][u][m].zcToken) returns (uint256 underlyingAmount) {
    Market memory market = markets[p][u][m];
    // if the market has not matured, mature it...
    if (market.maturityRate == 0) {
      if (!matureMarket(p, u, m)) { revert Exception(30, 0, 0, address(0), address(0)); }
      if (!IZcToken(market.zcToken).burn(f, a)) { revert Exception(29, 0, 0, address(0), address(0)); }
      ISwivel(swivel).authRedeem(p, u, market.cTokenAddr, t, a);
      return (a);
    } else {

      if (!IZcToken(market.zcToken).burn(f, a)) { revert Exception(29, 0, 0, address(0), address(0)); }
      uint256 amount = calculateReturn(p, u, m, a);
      ISwivel(swivel).authRedeem(p, u, market.cTokenAddr, t, amount);
      return (amount);
    }
  }

File: ZcToken.sol line 43-48

    function convertToUnderlying(uint256 principalAmount) external override view returns (uint256 underlyingAmount){
        if (block.timestamp < maturity) {
            return 0;
        }
        return (principalAmount * IRedeemer(redeemer).getExchangeRate(protocol, cToken) / IRedeemer(redeemer).markets(protocol, underlying, maturity).maturityRate);
    }

File: ZcToken.sol line 52-57

    function convertToPrincipal(uint256 underlyingAmount) external override view returns (uint256 principalAmount){
        if (block.timestamp < maturity) {
            return 0;
        }
        return (underlyingAmount * IRedeemer(redeemer).markets(protocol, underlying, maturity).maturityRate / IRedeemer(redeemer).getExchangeRate(protocol, cToken));
    }

File: ZcToken.sol line 61-66

    function maxRedeem(address owner) external override view returns (uint256 maxPrincipalAmount){
        if (block.timestamp < maturity) {
            return 0;
        }
        return (balanceOf[owner]);
    }

File: ZcToken.sol line 70-75 File: ZcToken.sol line 79-84 File: ZcToken.sol line 88-93

Open TODOs

File: Swivel.sol line 33

  address public aaveAddr; // TODO immutable?

File: Swivel.sol line 120

    // TODO cheaper to assign amount here or keep the ADD?

File: Swivel.sol line 157

    // TODO assign amount or keep the ADD?

File: Swivel.sol line 192

    // TODO assign amount or keep the ADD?

File: Swivel.sol line 221

    // TODO assign amount or keep ADD?

File: Swivel.sol line 707-708

    // TODO as stated elsewhere, we may choose to simply return true in all and not attempt to measure against any expected return
    if (p == uint8(Protocols.Compound)) { // TODO is Rari a drop in here?

File: Swivel.sol line 286 File: Swivel.sol line 317 File: Swivel.sol line 347 File: Swivel.sol line 716 File: Swivel.sol line 721 File: Swivel.sol line 748

Missing checks for address(0x0) when assigning values to address state variables

File: VaultTracker.sol line 35

    cTokenAddr = c;

[robrobbins]

open todos are all closed as per #200

[robrobbins]

it's not required to have ///@return in natspec as both the methods and the interfaces explicitly state them