Liquid Staking

Github

• https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/OperatorStakingPool.sol#L176

Summary

There is a potential inefficiency in the removeOperators function of the OperatorStakingPool contract. Specifically, the absence of a break statement within the inner loop causes unnecessary iterations even after an operator has been found and removed from the operators array. This could lead to increased gas costs, especially when dealing with large operator lists, and introduce logical redundancy. The report explains the problem, assesses its impact, and provides recommendations to optimize the function.

Vulnerability Details

See the following code:

The current implementation of the inner loop does not exit after finding and removing the operator. Without a break statement, the loop continues to iterate through the entire list of operators, which is unnecessary once the desired operator is found and removed. This can lead to excessive gas consumption when working with long operator lists. Potential logical redundancy as each iteration serves no purpose after the operator is removed. Handling duplicates incorrectly, as multiple occurrences of the same operator would be removed in one call.

Example Scenario

Assume the operators array has the following addresses:

Now, the owner calls removeOperators([0xC3]) to remove the operator with the address 0xC3.
Without a break statement:

The loop starts and checks:

• Index 0: operators[0] = 0xA1 → Not a match.

• Index 1: operators[1] = 0xB2 → Not a match.

• Index 2: operators[2] = 0xC3 → Match found! Operator removed.

Replace 0xC3 with the last operator 0xD4. Array becomes like [0xA1, 0xB2, 0xD4] and numOperators is reduced to 3.

However, the loop continues to the next iteration, even though the target operator was already removed. The loop then checks Index 3 (which now no longer exists after pop()), causes wasted gas. This redundant iteration continues through unnecessary elements, consuming extra gas.

Impact

Every iteration of the loop consumes gas. Continuing to iterate after removing the operator wastes gas, making the function expensive in scenarios with large operator lists.

Tools Used

Manual Review

Recommendations

Add a break statement within the inner loop to exit immediately after the operator is found and removed.