[MarketCapSqrtController] - The arguments should match the saved types

[cleanunicorn]

Description

The function prepareIndexPool accepts categoryID and indexSize as arguments.

  function prepareIndexPool(
    uint256 categoryID,
    uint256 indexSize,
    uint256 initialWethValue,
    string calldata name,
    string calldata symbol
  )

These arguments, along with name and symbol, are used to create a new index pool.

    poolAddress = _factory.deployIndexPool(
      keccak256(abi.encodePacked(categoryID, indexSize)),
      name,
      symbol
    );

After the pool is created, the new pool address is saved in a registry.

    _poolMeta[poolAddress] = IndexPoolMeta({
      categoryID: uint8(categoryID),
      indexSize: uint8(indexSize),
      initialized: false
    });

When the save happens, they are cast to uint8. However, this type does not match for IndexPoolMeta.categoryID, its defined type is uint16.

  struct IndexPoolMeta {
    uint16 categoryID;
    uint8 indexSize;
    bool initialized;
  }

Changing the type of the arguments of categoryID and indexSize to uint16 and respectively uint8 will also change the value of abi.encodePacked(categoryID, indexSize). This can create unintended consequences because other contracts might rely on a predictable address.

These arguments are also used in other functions and could create other unintended consequences or might need other parts of the system to be changed to accommodate new types.

Even though the uint8 type does not create issues, because the accommodating type is bigger, uint16 should match the one defined in the structure.

It's also important to note why the types are not uint256 in the first place; they can be "packed" by Solidity in one storage slot, thus saving space and reducing gas consumption.

Recommendation

When adding the pool registry information, one should cast categoryID to uint16.

i.e.

    _poolMeta[poolAddress] = IndexPoolMeta({
      categoryID: uint16(categoryID),
      indexSize: uint8(indexSize),
      initialized: false
    });

Another way to go about this is to save categoryID and indexSize as uint256, increasing gas cost but reducing the need to juggle types.

[d1ll0n]

Addressed as part of indexed-finance/indexed-core#35

[cleanunicorn]

👍