Submission Details
Severity:
If there is only one or two players in an entry, you should exclude them from being put through the for loops to check for duplicates
Summary
For loops are expensive and nested for loops are even more expensive, and it is not necessary to run the nested for loops to check for duplicates if you have only one or two players in entering in one entry.
Vulnerability Details
for (uint256 i = 0; i < players.length - 1; i++) {
for (uint256 j = i + 1; j < players.length; j++) {
require(players[i] != players[j], "PuppyRaffle: Duplicate player");
}
Impact
You are wasting gas and using unnecessary complexity in your code.
Tools Used
Manual review
Recommendations
Change the code as follows (note I am also using the custom error I recommended in another finding):
if(players.length == 1 or players.length == 2) {
if(players.length == 1) {return};
else{
if(player[0] == player[1]) {
revert PuppyRaffle__DuplicatePlayer()};}
else {return};}
else {
for (uint256 i = 0; i < players.length - 1; i++) {
for (uint256 j = i + 1; j < players.length; j++) {
if(players[i] == players[j]) {
revert PuppyRaffle__DuplicatePlayer()};
}
Updates
Rewards breakdown
nSLOC
143This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.