First Flight #10: One Shot
First Flight #10
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Unused variable in `RapBattle::_battle` function

bube

Summary

The totalPrize variable within the RapBattle::_battle function is declared and calculated but never used in any meaningful way. This variable is intended to represent the sum of the defenderBet and the challenger's _credBet.

Vulnerability Details

The variable totalPrize is initialized in RapBattle::_battle function but is never used.

function _battle(uint256 _tokenId, uint256 _credBet) internal {
address _defender = defender;
require(defenderBet == _credBet, "RapBattle: Bet amounts do not match");
uint256 defenderRapperSkill = getRapperSkill(defenderTokenId);
uint256 challengerRapperSkill = getRapperSkill(_tokenId);
uint256 totalBattleSkill = defenderRapperSkill + challengerRapperSkill;
@> uint256 totalPrize = defenderBet + _credBet;
​
uint256 random =
uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, msg.sender))) % totalBattleSkill;
​
// Reset the defender
defender = address(0);
emit Battle(msg.sender, _tokenId, random < defenderRapperSkill ? _defender : msg.sender);
​
// If random <= defenderRapperSkill -> defenderRapperSkill wins, otherwise they lose
if (random <= defenderRapperSkill) {
// We give them the money the defender deposited, and the challenger's bet
credToken.transfer(_defender, defenderBet);
credToken.transferFrom(msg.sender, _defender, _credBet);
} else {
// Otherwise, since the challenger never sent us the money, we just give the money in the contract
credToken.transfer(msg.sender, _credBet);
}
@> totalPrize = 0;
// Return the defender's NFT
oneShotNft.transferFrom(address(this), _defender, defenderTokenId);
}
​

First the totalPrize is assigned to be the sum of the defenderBet and _credBet. Then the variable is set to 0. That is not necessary, because the variable is local and after the execution of the _battle function, the value of the totalPrize variable will be cleared.

Impact

The presence of an unused variable contributes to code redundancy and can lead to confusion for anyone reviewing or maintaining the code.
Also, although minimal, the assignment and manipulation of an unused variable consume gas, leading to a slight inefficiency in contract execution.

Tools Used

Manual Review

Recommendations

Since totalPrize is not used to affect the contract's state or the outcome of the battle, it should be removed from the _battle function to clean up the code.

Updates

Lead Judging Commences

inallhonesty Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!