MyCut
First Flight #23
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

High Gas Costs in Pot Constructor for Large Player Arrays

Hoover

Summary

Excessive gas consumption from Pot.sol constructor called from ContestManager.sol::createContest when deploying a new Pot.sol with a large number of players.

Vulnerability Details

Affected code - https://github.com/Cyfrin/2024-08-MyCut/blob/946231db0fe717039429a11706717be568d03b54/src/Pot.sol#L32-L34

The constructor takes in an array of players that are saved as a state variable, and It's length is used to itterate through the playersToRewards[i_players[i]] = i_rewards[i];. With this looping condition it is calling the state variable each time and is using a lot of unnessesary gas.

address[] private i_players;
​
constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
i_players = players;
...
@> for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
}

PoC

  1. Generate two arrays consisting of 100 and 1000 players

  2. Calling creatingContest with 100 players and 1000 players

  3. Recording gas costs before and after

  4. We observe that the gas cost has risen significantly

function testGasCostForLargePlayerCount() public mintAndApproveTokens {
// Generate 100 players
address[] memory players100 = new address[](100);
uint256[] memory rewards100 = new uint256[](100);
for (uint256 i = 0; i < 100; i++) {
players100[i] = address(uint160(i + 1));
rewards100[i] = 1 ether;
}
​
// Generate 1000 players
address[] memory players1000 = new address[](1000);
uint256[] memory rewards1000 = new uint256[](1000);
for (uint256 i = 0; i < 1000; i++) {
players1000[i] = address(uint160(i + 1));
rewards1000[i] = 1 ether;
}
​
// Measure gas for 100 players
uint256 gasBefore100 = gasleft();
vm.startPrank(user);
ContestManager(conMan).createContest(players100, rewards100, IERC20(ERC20Mock(weth)), 100 ether);
vm.stopPrank();
uint256 gasUsed100 = gasBefore100 - gasleft();
​
// Measure gas for 1000 players
uint256 gasBefore1000 = gasleft();
vm.startPrank(user);
ContestManager(conMan).createContest(players1000, rewards1000, IERC20(ERC20Mock(weth)), 1000 ether);
vm.stopPrank();
​
uint256 gasUsed1000 = gasBefore1000 - gasleft();
​
console.log("Gas used for 100 players:", gasUsed100);
console.log("Gas used for 1000 players:", gasUsed1000);
}
Gas used for 100 players: 7324346
Gas used for 1000 players: 68756052

Impact

The unnecessary gas consumption in the Pot.sol constructor increases deployment costs, making it financially prohibitive for creating contest with a substantial large numbers of players. This inefficiency poses scalability challenges, limiting the number of players that can be feasibly included in a contest and potentially affecting user engagement.

Tools Used

Manual Review and Foundry Test

Recommendations

Cache the lengths of the storage arrays if they are used and not modified in for loops.

It is more gas efficient to cache it in some local variable and use that variable instead, like in the following example:

- for (uint256 i = 0; i < i_players.length; i++) {
- playersToRewards[i_players[i]] = i_rewards[i];
- }
-
+ uint256 players_length = i_players.length
+
+ for (uint256 i = 0; i < players_length; i++) {
+ playersToRewards[i_players[i]] = i_rewards[i];
+ }
Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Unbound for loop in Contest Creation

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.