Section 4 Puppy Raffle - Refund does not reduce the players.length making fees calc incorrect

[0xstormblessed]

Hey all,

not sure if this is the best place to post a possible issue, lmk if this is a known issue, i could not find it on the search.

So i found this issue while doing a quick review on PuppyRaffle following the video, not sure is expected that the PuppyRaffle may have further issues after the section, but this one looked like it was worth noticing.

PuppyRaffle:refund overwrites the player position to address(0) so in solidity the array length will remain the same and will not be reduced.

This may affect who wins the actual raffle since it could be an address 0 of a previously ocuppied slot in the array potentially and also definitely makes the fees calculation incorrect.

POC
Note: added a helper view function puppyRaffle.getPlayersLength() to assert the players.length from test.

    function testWithdrawFeesIncorrect_POC() public playersEntered {
        // player one quits!
        uint256 indexOfPlayer = puppyRaffle.getActivePlayerIndex(playerOne);
        vm.prank(playerOne);
        puppyRaffle.refund(indexOfPlayer);
        
        // still 4!!
        assertEq(puppyRaffle.getPlayersLength(),4);

        // player one enters again
        address[] memory players = new address[](1);
        players[0] = playerOne;
        puppyRaffle.enterRaffle{value: entranceFee * 1}(players);

        // now its 5!!
        assertEq(puppyRaffle.getPlayersLength(),5);

        vm.warp(block.timestamp + duration + 1);
        vm.roll(block.number + 1);

        uint256 expectedPrizeAmount = ((entranceFee * 4) * 20) / 100;

        puppyRaffle.selectWinner();
        // totalFees = 1000000000000000000 != expectedPrizeAmount = 800000000000000000
        assert(puppyRaffle.totalFees() != expectedPrizeAmount);
    }

[EngrPips]

Hello @0xstormblessed, Thanks for pointing this out, and it is expected that we may find more bugs in the codebase than what was touched on in the course. Patrick can't go over every single bug that the codebase may contain.

[0xstormblessed]

Thanks for the response, i guess this can be closed then.

[EngrPips]

You are welcome.