QA Report

[code423n4]

QA Report

[L-01] Add constructor initializers

As per OpenZeppelin’s (OZ) recommendation, “The guidelines are now to make it impossible for anyone to run initialize on an implementation contract, by adding an empty constructor with the initializer modifier. So the implementation contract gets initialized automatically upon deployment.”

Note that this behaviour is also incorporated the OZ Wizard since the UUPS vulnerability discovery: “Additionally, we modified the code generated by the Wizard 19 to include a constructor that automatically initializes the implementation when deployed.”

Furthermore, this thwarts any attempts to frontrun the initialization tx of these contracts:

contracts/ManagedIndex.sol:
  27:     function initialize(address[] calldata _assets, uint8[] calldata _weights) external {

contracts/TopNMarketCapIndex.sol:
  37:     function initialize(

contracts/TrackedIndex.sol:
  25:     function initialize(

contracts/vToken.sol:
  55:     function initialize(address _asset, address _registry) external override initializer {

[L-02] require() should be used for checking error conditions on inputs and return values while assert() should be used for invariant checking

Properly functioning code should never reach a failing assert statement, unless there is a bug in your contract you should fix. Here, I believe the assert should be a require or a revert:

IndexLogic.sol:72:        assert(minAmountInBase != type(uint).max);

As the Solidity version is > 0.8.0 the remaining gas would still be refunded in case of failure.

[L-03] Prevent accidentally burning tokens

Transferring tokens to the zero address is usually prohibited to accidentally avoid "burning" tokens by sending them to an unrecoverable zero address.

Consider adding a check to prevent accidentally burning tokens here:

vToken.sol:210:        _NAV.transfer(_from, _to, _amount);

[N-01] Avoid floating pragmas: the version should be locked

The declared pragma in the solution is pragma solidity >=0.8.7; but this should be locked.

[N-02] Missing comments

The following comments are missing (see @audit tags):

contracts/interfaces/IFeePool.sol:
   8      /// @notice Minting fee in base point format
   9:     /// @return Returns minting fee in base point (BP) format  //@audit NC: missing @param _index
  10      function mintingFeeInBPOf(address _index) external view returns (uint16);
  11  
  12      /// @notice Burning fee in base point format
  13:     /// @return Returns burning fee in base point (BP) format  //@audit NC: missing @param _index
  14      function burningFeeInBPOf(address _index) external view returns (uint16);
  15  
  16      /// @notice AUM scaled per seconds rate
  17:     /// @return Returns AUM scaled per seconds rate  //@audit NC: missing @param _index
  18      function AUMScaledPerSecondsRateOf(address _index) external view returns (uint);

contracts/interfaces/IPriceOracle.sol:
   8      /// @notice Updates and returns asset per base
   9:     /// @return Asset per base in UQ  //@audit NC: missing @param _asset
  10      function refreshedAssetPerBaseInUQ(address _asset) external returns (uint);
  11  
  12      /// @notice Returns last asset per base
  13:     /// @return Asset per base in UQ  //@audit NC: missing @param _asset
  14      function lastAssetPerBaseInUQ(address _asset) external view returns (uint);

contracts/interfaces/IvToken.sol:
  72      /// @notice Returns amount of assets for the given account with the given shares amount 
  73:     /// @return Amount of assets for the given account with the given shares amount  //@audit NC: missing @param * 2
  74      function assetDataOf(address _account, uint _shares) external view returns (AssetData memory);

contracts/interfaces/IvTokenFactory.sol:
  8      /// @notice Creates or returns address of previously created vToken for the given asset
  9:     /// @param _asset Asset to create or return vToken for  //@audit NC: missing @return address
  10      function createOrReturnVTokenOf(address _asset) external returns (address);

contracts/libraries/NAV.sol:
  18      /// @notice Transfer `_amount` of shares between given addresses
  19:     /// @param _from Account to send shares from //@audit NC: missing @param self
  20      /// @param _to Account to send shares to
  21      /// @param _amount Amount of shares to send
  22      function transfer(

  32      /// @notice Mints shares to the `_recipient` account
  33      /// @param self Data structure reference
  34      /// @param _balance New shares maximum limit
  35:     /// @param _recipient Recipient that will receive minted shares //@audit NC: missing @return shares
  36      function mint(

  53      /// @notice Burns shares from the `_recipient` account
  54      /// @param self Data structure reference
  55:     /// @param _balance Shares balance //@audit NC: missing @return amount
  56      function burn(Data storage self, uint _balance) internal returns (uint amount) {

[moose-code]

Good stuff, neatly presented :)