TwentyOne
First Flight #29
Beginner FriendlyGameFiFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

Higher GAS COST consumed in `TwentyOne::playersHand` & `TwentyOne::dealersHand` functions, mainly due to for loop usage.

affanimran1303

Summary:
TwentyOne::playersHand & TwnetyOne::dealersHand calculates the total value of player and dealer hands respectively. However in these two functions for loop has been used, which results in HIGH GAS cost, since it has to iterate through all the players. And as the number of players increases, iteration will increase which will result in HIGHER GAS.

Vulnerability Details:

TwentyOne::playersHand using for loop:

function playersHand(address player) public view returns (uint256) {
uint256 playerTotal = 0;
​
//Use of for loop, resulting in HIGH GAS COST
for (uint256 i = 0; i < playersDeck[player].playersCards.length; i++) {
uint256 cardValue = playersDeck[player].playersCards[i] % 13;
​
if (cardValue == 0 || cardValue >= 10) {
playerTotal += 10;
} else {
playerTotal += cardValue;
}
}
return playerTotal;
}

TwentyOne::dealersHand using for loop:

function dealersHand(address player) public view returns (uint256) {
uint256 dealerTotal = 0;
​
//Use of for loop, resulting in HIGH GAS COST
​
for (uint256 i = 0; i < dealersDeck[player].dealersCards.length; i++) {
uint256 cardValue = dealersDeck[player].dealersCards[i] % 13;
if (cardValue >= 10) {
dealerTotal += 10;
} else {
dealerTotal += cardValue;
}
}
return dealerTotal;
}

Test demonstrating vulnerability:

function test_PlayersHandGasComparison()public{
vm.startPrank(player1);
​
twentyOne.startGame{value: 1 ether}();
twentyOne.hit();
twentyOne.hit();
​
uint256 expectedTotal = twentyOne.playersHand(player1);
uint256 gasBeforeLoop=gasleft();
uint256 totalUsingLoop=twentyOne.playersHand(player1);
uint256 gasAfterLoop=gasleft();
​
console.log('Gas Used (For Loop):',gasBeforeLoop-gasAfterLoop);
​
precomputedTotals[player1] = expectedTotal;
uint256 gasBeforeMapping = gasleft();
uint256 totalUsingMapping = precomputedTotals[player1];
uint256 gasAfterMapping = gasleft();
​
console.log("Gas Used (Precomputed):",gasAfterMapping-gasAfterMapping);
​
assertEq(totalUsingLoop,expectedTotal,"For loop calculation mismatch");
assertEq(totalUsingMapping,expectedTotal,"Precomputed total mismatch");
​
assertLt(
(gasBeforeLoop-gasAfterLoop),
(gasBeforeMapping-gasAfterMapping),
"Precomputed total should be cheaper!"
);
vm.stopPrank();
}
function test_DealersHandGasComparison()public{
vm.startPrank(player1);
​
twentyOne.startGame{value: 1 ether}();
twentyOne.hit();
twentyOne.hit();
​
uint256 expectedTotal = twentyOne.dealersHand(player1);
​
uint256 gasBeforeLoop=gasleft();
uint256 totalUsingLoop=twentyOne.dealersHand(player1);
uint256 gasAfterLoop=gasleft();
​
console.log('Gas Used (For Loop):',gasBeforeLoop-gasAfterLoop);
​
precomputedTotals[player1] = expectedTotal;
uint256 gasBeforeMapping = gasleft();
uint256 totalUsingMapping = precomputedTotals[player1];
uint256 gasAfterMapping = gasleft();
​
console.log("Gas Used (Precomputed):",gasAfterMapping-gasAfterMapping);
​
assertEq(totalUsingLoop,expectedTotal,"For loop calculation mismatch");
assertEq(totalUsingMapping,expectedTotal,"Precomputed total mismatch");
​
assertLt(
(gasBeforeLoop-gasAfterLoop),
(gasBeforeMapping-gasAfterMapping),
"Precomputed total should be cheaper!"
);
vm.stopPrank();
}

Running these test functions will show the HIGH GAS output.

Impact:

As the number of cards in the player's hand increases, iteration in for loop increases and consumes more gas, resulting in the following impact:

  • High Gas fees, creating a barrier to entry, for new users.

  • Transactions could approach or exceed the block gas limit, creating a DoS situation, where users cannot complete their game or claim their winnings.

Tools Used:

Manual Testing, Foundry

Recommendations:

  1. Use of a mapping-based approach or precomputed totals instead of iterating through arrays in TwentyOne::playersHand & TwentyOne::dealersHand

  2. Impose a limit on the number of cards a player can hold to prevent excessive iterations or HIGH GAS usage scenarios.

First, optimize TwentyOne::playersHand function:

function playersHand(address player) public view returns (uint256) {
return precomputedTotals[player];
}

Then, add new logic for Precomputing Totals by updating addCardForPlayer, maintaining a running total in precomputedTotals:

function addCardForPlayer(address player, uint256 card) internal {
uint256 cardValue = card % 13;
uint256 valueToAdd = (cardValue == 0 || cardValue >= 10) ? 10 : cardValue;
​
playersDeck[player].playersCards.push(card);
precomputedTotals[player] += valueToAdd; // Update the running total
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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