Summary
Unnecessary double check of status in collectPresent
for someone to collect their present, waste of gas
Vulnerability Details
SantasList::collectPresent
is checking users to be NICE
OR EXTRA NICE
in both s_theListCheckedOnce
& s_theListCheckedTwice
which is not required, we can only check in s_theListCheckedTwice
because if a user is NICE
OR EXTRA NICE
in second checklist then it means user is also NICE
OR EXTRA NICE
in first checklist, as while adding user in second checklist first checklist is checked
function checkTwice(address person, Status status) external onlySanta {
@> if (s_theListCheckedOnce[person] != status) {
revert SantasList__SecondCheckDoesntMatchFirst();
}
s_theListCheckedTwice[person] = status;
emit CheckedTwice(person, status);
}
Impact
Waste of gas
Before gas = 70256
After gas = 70022
function test_checkGas() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.NICE);
santasList.checkTwice(user, SantasList.Status.NICE);
vm.stopPrank();
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
vm.startPrank(user);
santasList.collectPresent();
}
Run above test with and without recomendation
forge test --mt test_checkGas --gas-report
Tools Used
Recommendations
Remove first check from collectPresent
- if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
_mintAndIncrement();
return;
} else if (
- s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE
- && s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
_mintAndIncrement();
i_santaToken.mint(msg.sender);
return;
}
+ if ( s_theListCheckedTwice[msg.sender] == Status.NICE) {
_mintAndIncrement();
return;
+ } else if ( s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE) {
_mintAndIncrement();
i_santaToken.mint(msg.sender);
return;
}