Summary
In the functions LSTRewardsSplitter::_splitRewards and LSTRewardsSplitter::_totalFeesBasisPoints, the length of the fees array is recalculated in each iteration of the for loop. This causes unnecessary gas consumption as the array length does not change during the loop execution.
Vulnerability Details
https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/lstRewardsSplitter/LSTRewardsSplitter.sol#L173
function _splitRewards(uint256 _rewardsAmount) private {
for (uint256 i = 0; i < fees.length; ++i) {
Fee memory fee = fees[i];
uint256 amount = (_rewardsAmount * fee.basisPoints) / 10000;
if (fee.receiver == address(lst)) {
IStakingPool(address(lst)).burn(amount);
} else {
lst.safeTransfer(fee.receiver, amount);
}
}
principalDeposits = lst.balanceOf(address(this));
emit RewardsSplit(_rewardsAmount);
}
https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/lstRewardsSplitter/LSTRewardsSplitter.sol#L193
function _totalFeesBasisPoints() private view returns (uint256) {
uint256 totalFees;
for (uint i = 0; i < fees.length; i++) {
totalFees += fees[i].basisPoints;
}
return totalFees;
}
Impact
Unoptimized gas usage leads to slightly higher transaction costs for users interacting with the contract, especially when splitting rewards or calculating fees.
Proof of Concept
Copy the following code and paste it in your remix IDE. It is a simple Solidity contract that demonstrates the gas efficiency between using fees.length directly inside a loop versus storing fees.length in a local variable (feesLength) to reduce gas consumption.
pragma solidity ^0.8.0;
contract GasEfficiencyTest {
uint256[] public fees;
constructor() {
for (uint256 i = 0; i < 100; i++) {
fees.push(i);
}
}
function inefficientLoop() external view returns (uint256) {
uint256 total = 0;
for (uint256 i = 0; i < fees.length; i++) {
total += fees[i];
}
return total;
}
function efficientLoop() external view returns (uint256) {
uint256 total = 0;
uint256 feesLength = fees.length;
for (uint256 i = 0; i < feesLength; i++) {
total += fees[i];
}
return total;
}
}
After deploying the contract, you'll have access to both inefficientLoop and efficientLoop.
Now do the following:
Go to the Remix "Deploy & Run Transactions" panel.
Call the inefficientLoop function and observe the gas used in the transaction details.
Call the efficientLoop function and compare the gas used to the previous result.
Tools Used
Recommendations
Cache the length of the array in a variable before the loop to avoid recalculating it multiple times:
function _splitRewards(uint256 _rewardsAmount) private {
+ uint256 feesLength = fees.length;
- for (uint256 i = 0; i < fees.length; ++i) {
+ for (uint256 i = 0; i < feesLength; ++i) {
Fee memory fee = fees[i];
uint256 amount = (_rewardsAmount * fee.basisPoints) / 10000;
if (fee.receiver == address(lst)) {
IStakingPool(address(lst)).burn(amount);
} else {
lst.safeTransfer(fee.receiver, amount);
}
}
principalDeposits = lst.balanceOf(address(this));
emit RewardsSplit(_rewardsAmount);
}
/**
* @notice Returns the sum of all fees
* @return sum of fees in basis points
**/
function _totalFeesBasisPoints() private view returns (uint256) {
uint256 totalFees;
+ uint256 feesLength = fees.length;
- for (uint256 i = 0; i < fees.length; ++i) {
+ for (uint i = 0; i < feesLength; i++) {
totalFees += fees[i].basisPoints;
}
return totalFees;
}
