Phuture Finance contest - IllIllI's results

Lines of code

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/vToken.sol#L85

Vulnerability details

Impact

The ORDERER_ROLE role has the ability to arbitrarily transfer user funds, and this role is shared between both the orderer and people who can rebalance the index.

Even if the owner is benevolent the fact that there is a rug vector available may negatively impact the protocol's reputation. See this example where a similar finding has been flagged as a high-severity issue. I've downgraded this instance to be a medium since it requires a malicious manager.

Proof of Concept

The role is given to the orderer so it has the ability to add/remove funds during Uniswap operations: File: contracts/vToken.sol (lines 80-87)

    /// @inheritdoc IvToken
    function transferFrom(
        address _from,
        address _to,
        uint _shares
    ) external override nonReentrant onlyRole(ORDERER_ROLE) {
        _transfer(_from, _to, _shares);
    }

The role is also required to initiate rebalances: File: contracts/TopNMarketCapIndex.sol (lines 67-68)

    /// @notice Reweighs index assets according to the latest market cap data for specified category
    function reweight() external override onlyRole(ORDERER_ROLE) {

File: contracts/TrackedIndex.sol (lines 56-57)

    /// @notice Reweighs index assets according to the latest market cap data
    function reweight() external override onlyRole(ORDERER_ROLE) {

It is not necessary for the person/tool initiating reweights to also have the ability to arbitrarily transfer funds, so they should be separate roles. If the orderer also needs to be able to reweight, the orderer should also be given the new role.

Tools Used

Code inspection

Recommended Mitigation Steps

Split the role into two, and only give the ORDERER_ROLE role to the orderer

Lines of code

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/ChainlinkPriceOracle.sol#L83-L84

Vulnerability details

Impact

Stale prices can lead to the incorrect valuation of assets

Proof of Concept

The code does not check the other data returned from latestRoundData() which must be used to ensure that the data is not stale and that the price is valid

File: contracts/ChainlinkPriceOracle.sol (lines 83-86)

        (, int basePrice, , , ) = baseAggregator.latestRoundData();
        (, int quotePrice, , , ) = assetInfo.aggregator.latestRoundData();

        require(basePrice > 0 && quotePrice > 0, "ChainlinkPriceOracle: NEGATIVE");

Tools Used

Code inspection

Recommended Mitigation Steps

In addition to checking that the prices is above zero, code that uses Chainlink data must check that the answeredInRound is greater than or equal to the expected one, and that the timestamp is non-zero, indicating that the round is complete

Lines of code

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/IndexLogic.sol#L127

Vulnerability details

Impact

If an index has any inactive assets with the role SKIPPED_ASSET_ROLE, a user can repeatedly deposit and withdraw assets, always getting the skipped asset without having to deposit any

Proof of Concept

During minting, any asset that has the 'skipped' role is excluded from the checks of assets deposited: File: contracts/IndexLogic.sol (lines 60-70)

        for (uint i; i < inactiveAssets.length(); ++i) {
            if (!IAccessControl(registry).hasRole(SKIPPED_ASSET_ROLE, inactiveAssets.at(i))) {
                uint lastBalanceInAsset = IvToken(
                    IvTokenFactory(vTokenFactory).createOrReturnVTokenOf(inactiveAssets.at(i))
                ).lastAssetBalanceOf(address(this));
                lastAssetBalanceInBase += lastBalanceInAsset.mulDiv(
                    FixedPoint112.Q112,
                    oracle.refreshedAssetPerBaseInUQ(inactiveAssets.at(i))
                );
            }
        }

During burning, however, there's a bug that only skips if there are 'blacklisted' assets: File: contracts/IndexLogic.sol (lines 125-140)

        for (uint i; i < length + inactiveAssets.length(); ++i) {
            address asset = i < length ? assets.at(i) : inactiveAssets.at(i - length);
            if (containsBlacklistedAssets && IAccessControl(registry).hasRole(SKIPPED_ASSET_ROLE, asset)) {
                continue;
            }

            IvToken vToken = IvToken(IvTokenFactory(vTokenFactory).vTokenOf(asset));
            uint indexAssetBalance = vToken.balanceOf(address(this));
            uint accountBalance = (value * indexAssetBalance) / totalSupply();
            if (accountBalance == 0) {
                continue;
            }

            // calculate index value in vault to be burned
            vToken.transfer(address(vToken), accountBalance);
            vToken.burn(_recipient);

This means that users will be passed back inactive skipped assets even if they never deposited any

Tools Used

Code inspection

Recommended Mitigation Steps

I believe the && was meant to be a || in the SKIPPED_ASSET_ROLE in the code block directly above. Changing the code to be that way would be the fix.

Low Risk Issues

require() should be used instead of assert()

1. File: contracts/IndexLogic.sol (line 72)

        assert(minAmountInBase != type(uint).max);

Incorrect comment

Transfers the current balance if there is less available, rather than the usual reverting

1. File: contracts/vToken.sol (line 216)

    /// @param _amount Amount of assets to transfer

Unbounded loops with external calls

The interface and the function should require a start index and a lenght, so that the index composition can be fetched in batches without running out of gas. If there are thousands of index components (e.g. like the Wilshire 5000 index), the function may revert

1. File: contracts/BaseIndex.sol (lines 75-81)

    function anatomy() external view override returns (address[] memory _assets, uint8[] memory _weights) {
        _assets = assets.values();
        _weights = new uint8[](_assets.length);
        for (uint i; i < _assets.length; ++i) {
            _weights[i] = weightOf[_assets[i]];
        }
    }

Insufficient input validation

Checking for length greater than one is useless because the caller can just pass a weighting of zero for the second asset in order to exclude it

1. File: contracts/ManagedIndexReweightingLogic.sol (line 30)

            _updatedAssets.length > 1 &&

Registries should have ability to have per-index overrides

If two indexes share the same registry, it's not possible to separately apply SKIPPED_ASSET_ROLE for one but not the other. It's not always clear during index creation whether there will be circumstances that affect one but not the other index

1. File: contracts/BaseIndex.sol (line 38)

        registry = IIndexFactory(_factory).registry();

Uniswap DOS

The README.md talks about the fact that the orderer splits up orders to reduce price impact. This means that either the orderer has a slippage bounds which can DOSed with sandwich attacks, or the code uses some sort of VWAP/TWAP, which can also be gamed with flash loans submitted for every slice of the order

1. File: contracts/IndexLogic.sol (line 142)

                IOrderer(orderer).reduceOrderAsset(asset, totalSupply() - value, totalSupply());

Non-critical Issues

Adding a return statement when the function defines a named return variable, is redundant

1. File: contracts/libraries/AUMCalculationLibrary.sol (line 71)

        return z_;

2. File: contracts/libraries/FullMath.sol (line 39)

                return result;

3. File: contracts/libraries/FullMath.sol (line 106)

            return result;

require()/revert() statements should have descriptive reason strings

1. File: contracts/libraries/FullMath.sol (line 35)

                require(denominator > 0);

2. File: contracts/libraries/FullMath.sol (line 44)

            require(denominator > prod1);

3. File: contracts/libraries/FullMath.sol (line 123)

                require(result < type(uint256).max);

constants should be defined rather than using magic numbers

1. File: contracts/IndexLogic.sol (line 82)

            _mint(address(0xdead), IndexLibrary.INITIAL_QUANTITY);

2. File: contracts/libraries/FullMath.sol (line 88)

            uint256 inv = (3 * denominator) ^ 2;

Use bit shifts in an imutable variable rather than long bit masks of a single bit, for readability

1. File: contracts/libraries/FixedPoint112.sol (line 10)

    uint256 internal constant Q112 = 0x10000000000000000000000000000;

Use a more recent version of solidity

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

1. File: contracts/ManagedIndex.sol (line 3)

pragma solidity >=0.8.7;

Variable names that consist of all capital letters should be reserved for const/immutable variables

If the variable needs to be different based on which class it comes from, a view/pure function should be used instead (e.g. like this).

1. File: contracts/ManagedIndex.sol (line 17)

    bytes32 private REWEIGHT_INDEX_ROLE;

2. File: contracts/vToken.sol (line 41)

    NAV.Data internal _NAV;

File is missing NatSpec

1. File: contracts/interfaces/external/IChainLinkFeed.sol (line 0)

// SPDX-License-Identifier: GPL-2.0-or-later

2. File: contracts/interfaces/external/IWETH.sol (line 0)

// SPDX-License-Identifier: GPL-2.0-or-later

NatSpec is incomplete

1. File: contracts/interfaces/IChainlinkPriceOracle.sol (lines 10-13)

    /// @notice Adds `_asset` to the oracle
    /// @param _asset Asset's address
    /// @param _asset Asset aggregator's address
    function addAsset(address _asset, address _assetAggregator) external;

Missing: @param _assetAggregator

2. File: contracts/interfaces/IFeePool.sol (lines 8-10)

    /// @notice Minting fee in base point format
    /// @return Returns minting fee in base point (BP) format
    function mintingFeeInBPOf(address _index) external view returns (uint16);

Missing: @param _index

3. File: contracts/interfaces/IFeePool.sol (lines 12-14)

    /// @notice Burning fee in base point format
    /// @return Returns burning fee in base point (BP) format
    function burningFeeInBPOf(address _index) external view returns (uint16);

Missing: @param _index

4. File: contracts/interfaces/IFeePool.sol (lines 16-18)

    /// @notice AUM scaled per seconds rate
    /// @return Returns AUM scaled per seconds rate
    function AUMScaledPerSecondsRateOf(address _index) external view returns (uint);

Missing: @param _index

5. File: contracts/interfaces/IPriceOracle.sol (lines 8-10)

    /// @notice Updates and returns asset per base
    /// @return Asset per base in UQ
    function refreshedAssetPerBaseInUQ(address _asset) external returns (uint);

Missing: @param _asset

6. File: contracts/interfaces/IPriceOracle.sol (lines 12-14)

    /// @notice Returns last asset per base
    /// @return Asset per base in UQ
    function lastAssetPerBaseInUQ(address _asset) external view returns (uint);

Missing: @param _asset

7. File: contracts/interfaces/IvTokenFactory.sol (lines 8-10)

    /// @notice Creates or returns address of previously created vToken for the given asset
    /// @param _asset Asset to create or return vToken for
    function createOrReturnVTokenOf(address _asset) external returns (address);

Missing: @return

8. File: contracts/interfaces/IvToken.sol (lines 72-74)

    /// @notice Returns amount of assets for the given account with the given shares amount
    /// @return Amount of assets for the given account with the given shares amount
    function assetDataOf(address _account, uint _shares) external view returns (AssetData memory);

Missing: @param _account @param _shares

9. File: contracts/libraries/NAV.sol (lines 18-26)

    /// @notice Transfer `_amount` of shares between given addresses
    /// @param _from Account to send shares from
    /// @param _to Account to send shares to
    /// @param _amount Amount of shares to send
    function transfer(
        Data storage self,
        address _from,
        address _to,
        uint _amount

Missing: @param self

10. File: contracts/libraries/NAV.sol (lines 34-40)

    /// @param _balance New shares maximum limit
    /// @param _recipient Recipient that will receive minted shares
    function mint(
        Data storage self,
        uint _balance,
        address _recipient
    ) internal returns (uint shares) {

Missing: @return

11. File: contracts/libraries/NAV.sol (lines 54-56)

    /// @param self Data structure reference
    /// @param _balance Shares balance
    function burn(Data storage self, uint _balance) internal returns (uint amount) {

Missing: @return

Event is missing indexed fields

Each event should use three indexed fields if there are three or more fields

1. File: contracts/interfaces/IAnatomyUpdater.sol (line 8)

    event UpdateAnatomy(address asset, uint8 weight);

2. File: contracts/interfaces/IvToken.sol (line 13)

    event VTokenTransfer(address indexed from, address indexed to, uint amount);

Typos

1. File: contracts/interfaces/IAnatomyUpdater.sol (line 6)

/// @notice Contains event for aatomy update

aatomy

2. File: contracts/interfaces/IReweightableIndex.sol (line 5)

/// @title Rewightable index interface

Rewightable

3. File: contracts/libraries/FullMath.sol (line 101)

            // correct result modulo 2**256. Since the precoditions guarantee

precoditions

4. File: contracts/vToken.sol (line 84) Why is this one named _shares whereas the others are named _amount

        uint _shares

Use of sensitive/non-inclusive terms

Rename to constainsBlockedAssets

1. File: contracts/IndexLogic.sol (line 101)

        bool containsBlacklistedAssets;

State variables only set in the constructor should be declared immutable

Avoids a Gsset (20000 gas)

1. File: contracts/ManagedIndex.sol (line 17)

    bytes32 private REWEIGHT_INDEX_ROLE;

2. File: contracts/PhuturePriceOracle.sol (line 24)

    address public base;

3. File: contracts/PhuturePriceOracle.sol (line 27)

    address public registry;

4. File: contracts/PhuturePriceOracle.sol (line 33)

    uint8 private baseDecimals;

State variables can be packed into fewer storage slots

If variables occupying the same slot are both written the same function or by the constructor, avoids a separate Gsset (20000 gas). Reads of the variables are also cheaper

1. File: contracts/PhuturePriceOracle.sol (line 24)

    address public base;

Variable ordering with 3 slots instead of the current 4: mapping(32):priceOracleOf, address(20):base, uint8(1):baseDecimals, address(20):registry

State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second access of a state variable within a function. Caching will replace each Gwarmaccess (100 gas) with a much cheaper stack read. Less obvious optimizations include having local storage variables of mappings within state variable mappings or mappings within state variable structs, having local storage variables of structs within mappings, or having local caches of state variable contracts/addresses.

1. File: contracts/PhuturePriceOracle.sol (line 84)

        return IPriceOracle(priceOracleOf[_asset]).refreshedAssetPerBaseInUQ(_asset);

2. File: contracts/PhuturePriceOracle.sol (line 94)

        return IPriceOracle(priceOracleOf[_asset]).lastAssetPerBaseInUQ(_asset);

3. File: contracts/UniswapV2PathPriceOracle.sol (line 35) (save asset pointer for next iteration of the loop)

            address asset = path[i + 1];

4. File: contracts/UniswapV2PathPriceOracle.sol (line 50) (save asset pointer for next iteration of the loop)

            address asset = path[i + 1];

5. File: contracts/UniswapV2PriceOracle.sol (line 51)

        uint32 timeElapsed = blockTimestamp - blockTimestampLast;

6. File: contracts/vToken.sol (line 219)

        IERC20(asset).safeTransfer(_recipient, Math.min(_amount, balance));

Result of static calls should be cached in stack variables rather than re-calling storage-touching functions

Caching will replace each Gwarmaccess (100 gas) with a much cheaper stack read.

1. File: contracts/IndexLogic.sol (line 41)

            if (weightOf[assets.at(i)] == 0) {

2. File: contracts/ManagedIndexReweightingLogic.sol (line 40)

            uint availableAssets = IvToken(IvTokenFactory(vTokenFactory).createOrReturnVTokenOf(assets.at(i)))

3. File: contracts/TopNMarketCapReweightingLogic.sol (line 39)

            uint availableAssets = IvToken(IvTokenFactory(vTokenFactory).createOrReturnVTokenOf(assets.at(i)))

x = x + y is cheaper than x += y

1. File: contracts/libraries/NAV.sol (line 28)

        self.balanceOf[_from] -= _amount;

2. File: contracts/libraries/NAV.sol (line 29)

        self.balanceOf[_to] += _amount;

<array>.length should not be looked up in every loop of a for-loop

Even memory arrays incur the overhead of bit tests and bit shifts to calculate the array length. Storage array length checks incur an extra Gwarmaccess (100 gas) PER-LOOP.

1. File: contracts/BaseIndex.sol (line 78)

        for (uint i; i < _assets.length; ++i) {

2. File: contracts/ManagedIndexReweightingLogic.sol (line 50)

        for (uint i; i < _updatedAssets.length; ++i) {

3. File: contracts/ManagedIndexReweightingLogic.sol (line 96)

        for (uint i; i < _inactiveAssets.length; ++i) {

4. File: contracts/ManagedIndex.sol (line 30)

        for (uint i; i < _assets.length; ++i) {

5. File: contracts/TopNMarketCapIndex.sol (line 48)

        for (uint i; i < _assets.length; ++i) {

6. File: contracts/TopNMarketCapReweightingLogic.sol (line 104)

        for (uint i; i < _inactiveAssets.length; ++i) {

7. File: contracts/TrackedIndex.sol (line 35)

        for (uint i; i < _assets.length; ++i) {

++i/i++ should be unchecked{++i}/unchecked{++i} when it is not possible for them to overflow, as is the case when used in for- and while-loops

1. File: contracts/BaseIndex.sol (line 78)

        for (uint i; i < _assets.length; ++i) {

2. File: contracts/IndexLogic.sol (line 39)

        for (uint i; i < assets.length(); ++i) {

3. File: contracts/IndexLogic.sol (line 60)

        for (uint i; i < inactiveAssets.length(); ++i) {

4. File: contracts/IndexLogic.sol (line 102)

        for (uint i; i < length; ++i) {

5. File: contracts/IndexLogic.sol (line 125)

        for (uint i; i < length + inactiveAssets.length(); ++i) {

6. File: contracts/ManagedIndexReweightingLogic.sol (line 38)

        for (uint i; i < assets.length(); ++i) {

7. File: contracts/ManagedIndexReweightingLogic.sol (line 50)

        for (uint i; i < _updatedAssets.length; ++i) {

8. File: contracts/ManagedIndexReweightingLogic.sol (line 96)

        for (uint i; i < _inactiveAssets.length; ++i) {

9. File: contracts/ManagedIndex.sol (line 30)

        for (uint i; i < _assets.length; ++i) {

10. File: contracts/TopNMarketCapIndex.sol (line 48)

        for (uint i; i < _assets.length; ++i) {

11. File: contracts/TopNMarketCapReweightingLogic.sol (line 37)

        for (uint i; i < assets.length(); ++i) {

12. File: contracts/TopNMarketCapReweightingLogic.sol (line 51)

        for (uint _i; _i < diff.assetCount; ++_i) {

13. File: contracts/TopNMarketCapReweightingLogic.sol (line 104)

        for (uint i; i < _inactiveAssets.length; ++i) {

14. File: contracts/TrackedIndexReweightingLogic.sol (line 37)

        for (uint i; i < assets.length(); ++i) {

15. File: contracts/TrackedIndexReweightingLogic.sol (line 66)

        for (uint i; i < assets.length(); ++i) {

16. File: contracts/TrackedIndex.sol (line 35)

        for (uint i; i < _assets.length; ++i) {

17. File: contracts/UniswapV2PathPriceOracle.sol (line 34)

        for (uint i = 0; i < path.length - 1; i++) {

18. File: contracts/UniswapV2PathPriceOracle.sol (line 49)

        for (uint i = 0; i < path.length - 1; i++) {

require()/revert() strings longer than 32 bytes cost extra gas

1. File: contracts/TopNMarketCapIndex.sol (line 74)

                revert("TopNMarketCapIndex: REWEIGH_FAILED");

2. File: contracts/TopNMarketCapReweightingLogic.sol (line 67)

                require(IAccessControl(registry).hasRole(ASSET_ROLE, asset), "TopNMarketCapIndex: INVALID_ASSET");

3. File: contracts/UniswapV2PathPriceOracle.sol (line 25)

        require(_oracles.length == _path.length - 1, "UniswapV2PathPriceOracle: ORACLES");

Not using the named return variables when a function returns, wastes deployment gas

1. File: contracts/vToken.sol (line 91)

        return _mint(msg.sender);

2. File: contracts/vToken.sol (line 96)

        return _burn(_recipient);

Using > 0 costs more gas than != 0 when used on a uint in a require() statement

1. File: contracts/IndexLogic.sol (line 76)

            require(lastAssetBalanceInBase > 0, "Index: INSUFFICIENT_AMOUNT");

2. File: contracts/IndexLogic.sol (line 98)

        require(value > 0, "Index: INSUFFICIENT_AMOUNT");

3. File: contracts/libraries/FullMath.sol (line 35)

                require(denominator > 0);

4. File: contracts/libraries/IndexLibrary.sol (line 29)

        require(_assetPerBaseInUQ > 0, "IndexLibrary: ORACLE");

5. File: contracts/libraries/NAV.sol (line 49)

        require(shares > 0, "NAV: INSUFFICIENT_AMOUNT");

6. File: contracts/libraries/NAV.sol (line 59)

        require(amount > 0, "NAV: INSUFFICIENT_SHARES_BURNED");

It costs more gas to initialize variables to zero than to let the default of zero be applied

1. File: contracts/UniswapV2PathPriceOracle.sol (line 34)

        for (uint i = 0; i < path.length - 1; i++) {

2. File: contracts/UniswapV2PathPriceOracle.sol (line 49)

        for (uint i = 0; i < path.length - 1; i++) {

++i costs less gas than ++i, especially when it's used in for-loops (--i/i-- too)

1. File: contracts/UniswapV2PathPriceOracle.sol (line 34)

        for (uint i = 0; i < path.length - 1; i++) {

2. File: contracts/UniswapV2PathPriceOracle.sol (line 49)

        for (uint i = 0; i < path.length - 1; i++) {

Splitting require() statements that use && saves gas

See this issue for an example

1. File: contracts/ChainlinkPriceOracle.sol (line 51)

        require(_baseAggregator != address(0) && _base != address(0), "ChainlinkPriceOracle: ZERO");

2. File: contracts/ChainlinkPriceOracle.sol (line 86)

        require(basePrice > 0 && quotePrice > 0, "ChainlinkPriceOracle: NEGATIVE");

3. File: contracts/ManagedIndexReweightingLogic.sol (lines 29-34)

        require(
            _updatedAssets.length > 1 &&
                _updatedWeights.length == _updatedAssets.length &&
                _updatedAssets.length <= IIndexRegistry(registry).maxComponents(),
            "ManagedIndex: INVALID"
        );

4. File: contracts/UniswapV2PriceOracle.sol (line 46)

        require(reserve0 != 0 && reserve1 != 0, "UniswapV2PriceOracle: RESERVES");

Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed

1. File: contracts/ChainlinkPriceOracle.sol (line 23)

        uint8 answerDecimals;

2. File: contracts/ChainlinkPriceOracle.sol (line 24)

        uint8 decimals;

3. File: contracts/ChainlinkPriceOracle.sol (line 38)

    uint8 private immutable baseDecimals;

4. File: contracts/ChainlinkPriceOracle.sol (line 41)

    uint8 private immutable baseAnswerDecimals;

5. File: contracts/interfaces/IAnatomyUpdater.sol (line 8)

    event UpdateAnatomy(address asset, uint8 weight);

6. File: contracts/interfaces/IFeePool.sol (line 10)

    function mintingFeeInBPOf(address _index) external view returns (uint16);

7. File: contracts/interfaces/IFeePool.sol (line 14)

    function burningFeeInBPOf(address _index) external view returns (uint16);

8. File: contracts/interfaces/IPhuturePriceOracle.sol (line 23)

    function convertToIndex(uint _baseAmount, uint8 _indexDecimals) external view returns (uint);

9. File: contracts/libraries/BP.sol (line 10)

    uint16 constant DECIMAL_FACTOR = 10_000;

10. File: contracts/libraries/FixedPoint112.sol (line 8)

    uint8 internal constant RESOLUTION = 112;

11. File: contracts/libraries/IndexLibrary.sol (line 17)

    uint8 constant MAX_WEIGHT = type(uint8).max;

12. File: contracts/libraries/IndexLibrary.sol (line 26)

        uint8 _weight,

13. File: contracts/ManagedIndexReweightingLogic.sol (line 54)

            uint8 newWeight = _updatedWeights[i];

14. File: contracts/ManagedIndexReweightingLogic.sol (line 66)

                uint8 prevWeight = weightOf[asset];

15. File: contracts/ManagedIndex.sol (line 32)

            uint8 weight = _weights[i];

16. File: contracts/PhuturePriceOracle.sol (line 33)

    uint8 private baseDecimals;

17. File: contracts/PhuturePriceOracle.sol (line 68)

    function convertToIndex(uint _baseAmount, uint8 _indexDecimals) external view override returns (uint) {

18. File: contracts/TopNMarketCapIndex.sol (line 21)

    uint8 public topN;

19. File: contracts/TopNMarketCapIndex.sol (line 38)

        uint8 _topN,

20. File: contracts/TopNMarketCapIndex.sol (line 47)

        uint8 _totalWeight;

21. File: contracts/TopNMarketCapIndex.sol (line 51)

            uint8 weight = _i == 0

22. File: contracts/TopNMarketCapReweightingLogic.sol (line 50)

        uint8 _totalWeight;

23. File: contracts/TopNMarketCapReweightingLogic.sol (line 69)

                    uint8 weight;

24. File: contracts/TrackedIndexReweightingLogic.sol (line 34)

        uint8 totalWeight;

25. File: contracts/TrackedIndexReweightingLogic.sol (line 45)

            uint8 weight = uint8((_capitalizations[i] * type(uint8).max) / _totalCapitalization);

26. File: contracts/TrackedIndex.sol (line 32)

        uint8 totalWeight;

27. File: contracts/TrackedIndex.sol (line 37)

            uint8 weight = uint8((_capitalizations[i] * type(uint8).max) / _totalCapitalization);

28. File: contracts/UniswapV2PriceOracle.sol (line 29)

    uint32 private blockTimestampLast;

29. File: contracts/UniswapV2PriceOracle.sol (line 43)

        uint112 reserve0;

30. File: contracts/UniswapV2PriceOracle.sol (line 44)

        uint112 reserve1;

31. File: contracts/UniswapV2PriceOracle.sol (line 50)

        (uint price0Cml, uint price1Cml, uint32 blockTimestamp) = address(_pair).currentCumulativePrices();

32. File: contracts/UniswapV2PriceOracle.sol (line 51)

        uint32 timeElapsed = blockTimestamp - blockTimestampLast;

33. File: contracts/UniswapV2PriceOracle.sol (line 62)

        (uint price0Cumulative, uint price1Cumulative, uint32 blockTimestamp) = address(pair).currentCumulativePrices();

34. File: contracts/UniswapV2PriceOracle.sol (line 63)

        uint32 timeElapsed = blockTimestamp - blockTimestampLast;

Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

See this issue for a detail description of the issue

1. File: contracts/BaseIndex.sol (line 25)

    bytes32 internal constant INDEX_MANAGER_ROLE = keccak256("INDEX_MANAGER_ROLE");

2. File: contracts/ChainlinkPriceOracle.sol (line 29)

    bytes32 private constant ASSET_MANAGER_ROLE = keccak256("ASSET_MANAGER_ROLE");

3. File: contracts/IndexLogic.sol (line 25)

    bytes32 internal constant ASSET_ROLE = keccak256("ASSET_ROLE");

4. File: contracts/IndexLogic.sol (line 27)

    bytes32 internal constant SKIPPED_ASSET_ROLE = keccak256("SKIPPED_ASSET_ROLE");

5. File: contracts/ManagedIndexReweightingLogic.sol (line 25)

    bytes32 internal constant ASSET_ROLE = keccak256("ASSET_ROLE");

6. File: contracts/PhuturePriceOracle.sol (line 21)

    bytes32 private constant ASSET_MANAGER_ROLE = keccak256("ASSET_MANAGER_ROLE");

7. File: contracts/TopNMarketCapIndex.sol (line 18)

    bytes32 internal constant ORDERER_ROLE = keccak256("ORDERER_ROLE");

8. File: contracts/TopNMarketCapReweightingLogic.sol (line 27)

    bytes32 internal constant ASSET_ROLE = keccak256("ASSET_ROLE");

9. File: contracts/TrackedIndexReweightingLogic.sol (line 25)

    bytes32 internal constant ASSET_ROLE = keccak256("ASSET_ROLE");

10. File: contracts/TrackedIndex.sol (line 17)

    bytes32 internal constant ORDERER_ROLE = keccak256("ORDERER_ROLE");

11. File: contracts/vToken.sol (line 27)

    bytes32 private constant INDEX_ROLE = keccak256("INDEX_ROLE");

12. File: contracts/vToken.sol (line 29)

    bytes32 private constant ORACLE_ROLE = keccak256("ORACLE_ROLE");

13. File: contracts/vToken.sol (line 31)

    bytes32 private constant ORDERER_ROLE = keccak256("ORDERER_ROLE");

14. File: contracts/vToken.sol (line 33)

    bytes32 private constant RESERVE_MANAGER_ROLE = keccak256("RESERVE_MANAGER_ROLE");

Duplicated require()/revert() checks should be refactored to a modifier or function

1. File: contracts/PhuturePriceOracle.sol (line 83)

        require(priceOracleOf[_asset] != address(0), "PhuturePriceOracle: UNSET");

require() or revert() statements that check input arguments should be at the top of the function

Checks that involve constants should come before checks that involve state variables

1. File: contracts/ChainlinkPriceOracle.sol (line 62)

        require(_asset != address(0), "ChainlinkPriceOracle: ZERO");

2. File: contracts/PhuturePriceOracle.sol (line 47)

        require(_base != address(0), "PhuturePriceOracle: ZERO");

3. File: contracts/vToken.sol (line 60)

        require(_asset != address(0), "vToken: ZERO");

Use custom errors rather than revert()/require() strings to save deployment gas

1. File: contracts/BaseIndex.sol (Various lines throughout the file)
2. File: contracts/ChainlinkPriceOracle.sol (Various lines throughout the file)
3. File: contracts/IndexLogic.sol (Various lines throughout the file)
4. File: contracts/libraries/FullMath.sol (Various lines throughout the file)
5. File: contracts/libraries/IndexLibrary.sol (Various lines throughout the file)
6. File: contracts/libraries/NAV.sol (Various lines throughout the file)
7. File: contracts/ManagedIndexReweightingLogic.sol (Various lines throughout the file)
8. File: contracts/ManagedIndex.sol (Various lines throughout the file)
9. File: contracts/PhuturePriceOracle.sol (Various lines throughout the file)
10. File: contracts/TopNMarketCapIndex.sol (Various lines throughout the file)
11. File: contracts/TopNMarketCapReweightingLogic.sol (Various lines throughout the file)
12. File: contracts/TrackedIndexReweightingLogic.sol (Various lines throughout the file)
13. File: contracts/TrackedIndex.sol (Various lines throughout the file)
14. File: contracts/UniswapV2PathPriceOracle.sol (Various lines throughout the file)
15. File: contracts/UniswapV2PriceOracle.sol (Various lines throughout the file)
16. File: contracts/vToken.sol (Various lines throughout the file)

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.

1. File: contracts/PhuturePriceOracle.sol (line 55)

    function setOracleOf(address _asset, address _oracle) external override onlyRole(ASSET_MANAGER_ROLE) {

2. File: contracts/PhuturePriceOracle.sol (line 62)

    function removeOracleOf(address _asset) external override onlyRole(ASSET_MANAGER_ROLE) {

3. File: contracts/TopNMarketCapIndex.sol (line 68)

    function reweight() external override onlyRole(ORDERER_ROLE) {

4. File: contracts/TrackedIndex.sol (line 57)

    function reweight() external override onlyRole(ORDERER_ROLE) {

5. File: contracts/vToken.sol (lines 81-85)

    function transferFrom(
        address _from,
        address _to,
        uint _shares
    ) external override nonReentrant onlyRole(ORDERER_ROLE) {

6. File: contracts/vToken.sol (line 90)

    function mint() external override nonReentrant onlyRole(INDEX_ROLE) returns (uint shares) {

7. File: contracts/vToken.sol (line 95)

    function burn(address _recipient) external override nonReentrant onlyRole(INDEX_ROLE) returns (uint amount) {

8. File: contracts/vToken.sol (line 100)

    function mintFor(address _recipient) external override nonReentrant onlyRole(ORDERER_ROLE) returns (uint) {

9. File: contracts/vToken.sol (line 105)

    function burnFor(address _recipient) external override nonReentrant onlyRole(ORDERER_ROLE) returns (uint) {

Use a more recent version of solidity

Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value

1. File: contracts/BaseIndex.sol (line 3)

pragma solidity >=0.8.7;

2. File: contracts/ChainlinkPriceOracle.sol (line 3)

pragma solidity >=0.8.7;

3. File: contracts/IndexLayout.sol (line 3)

pragma solidity >=0.8.7;

4. File: contracts/IndexLogic.sol (line 3)

pragma solidity >=0.8.7;

5. File: contracts/interfaces/external/IChainLinkFeed.sol (line 3)

pragma solidity >=0.8.7;

6. File: contracts/interfaces/external/IWETH.sol (line 3)

pragma solidity >=0.8.7;

7. File: contracts/interfaces/IAnatomyUpdater.sol (line 3)

pragma solidity >=0.8.7;

8. File: contracts/interfaces/IChainlinkPriceOracle.sol (line 3)

pragma solidity >=0.8.7;

9. File: contracts/interfaces/IFeePool.sol (line 3)

pragma solidity >=0.8.7;

10. File: contracts/interfaces/IIndexFactory.sol (line 3)

pragma solidity >=0.8.7;

11. File: contracts/interfaces/IIndexLayout.sol (line 3)

pragma solidity >=0.8.7;

12. File: contracts/interfaces/IIndexLogic.sol (line 3)

pragma solidity >=0.8.7;

13. File: contracts/interfaces/IIndexRegistry.sol (line 3)

pragma solidity >=0.8.7;

14. File: contracts/interfaces/IIndex.sol (line 3)

pragma solidity >=0.8.7;

15. File: contracts/interfaces/IManagedIndexReweightingLogic.sol (line 3)

pragma solidity >=0.8.7;

16. File: contracts/interfaces/IManagedIndex.sol (line 3)

pragma solidity >=0.8.7;

17. File: contracts/interfaces/INameRegistry.sol (line 3)

pragma solidity >=0.8.7;

18. File: contracts/interfaces/IOrderer.sol (line 3)

pragma solidity >=0.8.7;

19. File: contracts/interfaces/IPhuturePriceOracle.sol (line 3)

pragma solidity >=0.8.7;

20. File: contracts/interfaces/IPriceOracle.sol (line 3)

pragma solidity >=0.8.7;

21. File: contracts/interfaces/IReweightableIndex.sol (line 3)

pragma solidity >=0.8.7;

22. File: contracts/interfaces/ITopNMarketCapCategories.sol (line 3)

pragma solidity >=0.8.7;

23. File: contracts/interfaces/ITopNMarketCapIndexFactory.sol (line 3)

pragma solidity >=0.8.7;

24. File: contracts/interfaces/ITopNMarketCapIndexReweightingLogic.sol (line 3)

pragma solidity >=0.8.7;

25. File: contracts/interfaces/ITrackedIndexReweightingLogic.sol (line 3)

pragma solidity >=0.8.7;

26. File: contracts/interfaces/IUniswapV2PathPriceOracle.sol (line 3)

pragma solidity >=0.8.7;

27. File: contracts/interfaces/IUniswapV2PriceOracle.sol (line 3)

pragma solidity >=0.8.7;

28. File: contracts/interfaces/IvTokenFactory.sol (line 3)

pragma solidity >=0.8.7;

29. File: contracts/interfaces/IvToken.sol (line 3)

pragma solidity >=0.8.7;

30. File: contracts/libraries/AUMCalculationLibrary.sol (line 3)

pragma solidity >=0.8.7;

31. File: contracts/libraries/BP.sol (line 3)

pragma solidity >=0.8.7;

32. File: contracts/libraries/FixedPoint112.sol (line 3)

pragma solidity >=0.8.7;

33. File: contracts/libraries/IndexLibrary.sol (line 3)

pragma solidity >=0.8.7;

34. File: contracts/libraries/NAV.sol (line 3)

pragma solidity >=0.8.7;

35. File: contracts/ManagedIndexReweightingLogic.sol (line 3)

pragma solidity >=0.8.7;

36. File: contracts/ManagedIndex.sol (line 3)

pragma solidity >=0.8.7;

37. File: contracts/PhutureIndex.sol (line 3)

pragma solidity >=0.8.7;

38. File: contracts/PhuturePriceOracle.sol (line 3)

pragma solidity >=0.8.7;

39. File: contracts/TopNMarketCapIndex.sol (line 3)

pragma solidity >=0.8.7;

40. File: contracts/TopNMarketCapReweightingLogic.sol (line 3)

pragma solidity >=0.8.7;

41. File: contracts/TrackedIndexReweightingLogic.sol (line 3)

pragma solidity >=0.8.7;

42. File: contracts/TrackedIndex.sol (line 3)

pragma solidity >=0.8.7;

43. File: contracts/UniswapV2PathPriceOracle.sol (line 3)

pragma solidity >=0.8.7;

44. File: contracts/UniswapV2PriceOracle.sol (line 3)

pragma solidity >=0.8.7;

45. File: contracts/vToken.sol (line 3)

pragma solidity >=0.8.7;