IndexPool Code Style Findings

IPL-01C: Deprecated Revert Pattern

Type	Severity	Location
Code Style		IndexPool.sol:L82

Description:

The referenced revert statement is deprecated as it yields a textual literal.

Example:

Copy
81if (heldTokens[i] == _tokenAddress) {
82    revert("IndexPool: Token has already been added");
83}

Recommendation:

We advise a proper error declaration to be introduced, or the if-revert pattern to be updated with the require pattern.

Alleviation (d639d227f8b8d90dbd9813ab9d7a5cbee34dd9b1):

A custom TokenCanNotBeAddedTwice error declaration was introduced for the relevant if-revert pattern addressing this exhibit.

IPL-02C: Inefficient Iterator Types

Type	Severity	Location
Gas Optimization		IndexPool.sol: • I-1: L48 • I-2: L51 • I-3: L80 • I-4: L95

Description:

The referenced for loop iterators are defined as unit8 data types inefficiently.

Example:

Copy
95for (uint8 i = 0; i < _tokens.length; i++) {

Recommendation:

The EVM is built to operate on 256-bit data types, and any types that utilize less bits involve masking operations that increase the gas cost involved on all operations.

We advise the relevant data types to be set to the uint256 data type, optimizing all instances that the variables are utilized in.

Alleviation (d639d227f8b8d90dbd9813ab9d7a5cbee34dd9b1):

All applicable iterator types have been updated to uint256 as advised, optimizing the code's gas cost.

IPL-03C: Inefficient Token Skipping Mechanism

Type	Severity	Location
Gas Optimization		IndexPool.sol:L44, L51-L56

Description:

The current token skipping mechanism is inefficient as it will perform an iteration of the _skipTokens array for each iteration of the heldTokens array.

Example:

Copy
42function burnIndexTokenAndSkipSomeTokens(
43    uint256 _amount,
44    address[] memory _skipTokens
45) public nonReentrant {
46    uint256 amountOfPoolBalance;
47    indexToken.burnFrom(msg.sender, _amount);
48    for (uint8 i = 0; i < heldTokens.length; i++) {
49        // Skip tokens that are in the skipTokens array
50        bool skipToken = false;
51        for (uint8 j = 0; j < _skipTokens.length; j++) {
52            if (heldTokens[i] == _skipTokens[j]) {
53                skipToken = true;
54                break;
55            }
56        }
57        if (skipToken) {
58            continue;
59        }
60
61        amountOfPoolBalance =
62            ((IERC20(heldTokens[i])).balanceOf(address(this)) * _amount) /
63            indexToken.totalSupply();
64        if (amountOfPoolBalance > 0) {
65            (IERC20(heldTokens[i])).safeTransfer(
66                msg.sender,
67                amountOfPoolBalance
68            );
69        }
70    }
71    emit IndexTokenBurned(msg.sender, _amount);
72}

Recommendation:

We advise the approach to be revised, mandating that callers configure the _skipTokens array in the same order that the tokens are identified in the heldTokens array.

This will permit the code to solely check the uppermost entry of the _skipTokens array, incrementing its index solely when a token has been skipped and thus greatly optimizing the function's gas cost and scalability.

Alleviation (d639d227f8b8d90dbd9813ab9d7a5cbee34dd9b1):

The optimization has been implemented as advised, greatly reducing the gas cost associated with skipping a particular token.

IPL-04C: Redundant Application of Modifier

Type	Severity	Location
Gas Optimization		IndexPool.sol:L78, L94

Description:

The IndexPool::addTokenBatch function will re-apply the Ownable::onlyOwner modifier once at the top-level call and once on each inner IndexPool::addToken call.

Example:

Copy
74/**
75 * @notice Adds a token to the index pool.
76 * @param _tokenAddress The address of the token to add.
77 */
78function addToken(address _tokenAddress) public onlyOwner {
79    // Check if the token is already in the pool, to prevent distributing it multiple times
80    for (uint8 i = 0; i < heldTokens.length; i++) {
81        if (heldTokens[i] == _tokenAddress) {
82            revert("IndexPool: Token has already been added");
83        }
84    }
85
86    heldTokens.push(_tokenAddress);
87    emit TokenAdded();
88}
89
90/**
91 * @notice Adds a batch of tokens to the index pool.
92 * @param _tokens The addresses of the tokens to add.
93 */
94function addTokenBatch(address[] calldata _tokens) external onlyOwner {
95    for (uint8 i = 0; i < _tokens.length; i++) {
96        addToken(_tokens[i]);
97    }
98}

Recommendation:

We advise the contents of the IndexPool::addToken function to be refactored to an internal function that does not apply the modifier, permitting the same function to be invoked across both implementations optimally.

Alleviation (d639d227f8):

This optimization has not been applied rendering it to be considered acknowledged. To note, both functions can remain public and the internalization of the IndexPool::addToken function's statements would not functionally affect either function.

Alleviation (2ef12e6885):

The optimization has been applied per our recommendation, greatly optimizing the gas cost of the batch function's operation.

IPL-05C: Redundant Parenthesis Statements

Type	Severity	Location
Code Style		IndexPool.sol: • I-1: L62 • I-2: L65

Description:

The referenced statements are redundantly wrapped in parenthesis' (()).

Example:

Copy
62((IERC20(heldTokens[i])).balanceOf(address(this)) * _amount) /

Recommendation:

We advise them to be safely omitted, increasing the legibility of the codebase.

Alleviation (d639d227f8b8d90dbd9813ab9d7a5cbee34dd9b1):

The redundant parenthesis in the referenced statements have been safely omitted.

IPL-06C: Variable Mutability Specifier (Immutable)

Type	Severity	Location
Gas Optimization		IndexPool.sol:L17

Description:

The linked variable is assigned to only once during the contract's constructor.

Example:

Copy
23constructor(address _indexToken) Ownable(msg.sender) {
24    indexToken = IERC20Burnable(_indexToken);
25}

Recommendation:

We advise it to be set as immutable greatly optimizing its read-access gas cost.

Alleviation (d639d227f8b8d90dbd9813ab9d7a5cbee34dd9b1):

The indexToken contract-level variable of the contract has been set as immutable, optimizing its read-access gas cost significantly.