Why does my test for overflow for totalFees in PuppyRaffle doesn't work?

[sharonphiliplima]

The below is my test:

function test_overflow_fees() public {
        address[] memory players = new address[](95);
        for (uint256 i = 0; i < 95; i++) {
            players[i] = address(i);
        }
        uint256 expectedFeeCollected = (entranceFee * 95) / 5;
        console2.log("expectedFeeCollected:", expectedFeeCollected);
        puppyRaffle.enterRaffle{value: entranceFee * 95}(players);

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

        puppyRaffle.selectWinner();
        uint256 actualFeeCollected = puppyRaffle.totalFees();

        assert(actualFeeCollected < expectedFeeCollected);
        console2.log("actualFeeCollected:", expectedFeeCollected);
    }

Output from console:

Ran 1 test for test/PuppyRaffleTest.t.sol:PuppyRaffleTest
[PASS] test_overflow_fees() (gas: 4797706)
Logs:
  expectedFeeCollected: 19000000000000000000
  actualFeeCollected: 19000000000000000000

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 39.60ms (34.23ms CPU time)

Ran 1 test suite in 61.41ms (39.60ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

It somehow passes but doesn't work

Meanwhile the test that Patrick provided:

function testTotalFeesOverflow() public playersEntered {
        // We finish a raffle of 4 to collect some fees
        vm.warp(block.timestamp + duration + 1);
        vm.roll(block.number + 1);
        puppyRaffle.selectWinner();
        uint256 startingTotalFees = puppyRaffle.totalFees();
        console.log("starting total fees", startingTotalFees);

        // startingTotalFees = 800000000000000000
        //800000000000000000
        // We then have 89 players enter a new raffle
        uint256 playersNum = 89;
        address[] memory players = new address[](playersNum);
        for (uint256 i = 0; i < playersNum; i++) {
            players[i] = address(i);
        }
        puppyRaffle.enterRaffle{value: entranceFee * playersNum}(players);
        // We end the raffle
        vm.warp(block.timestamp + duration + 1);
        vm.roll(block.number + 1);

        // And here is where the issue occurs
        // We will now have fewer fees even though we just finished a second raffle
        puppyRaffle.selectWinner();

        uint256 endingTotalFees = puppyRaffle.totalFees();
        console.log("ending total fees", endingTotalFees);
        //153255926290448384

        assert(endingTotalFees < startingTotalFees);

        // We are also unable to withdraw any fees because of the require check
        vm.prank(puppyRaffle.feeAddress());
        vm.expectRevert("PuppyRaffle: There are currently players active!");
        puppyRaffle.withdrawFees();
    }

Console log output:

Ran 1 test for test/PuppyRaffleTest.t.sol:PuppyRaffleTest
[PASS] testTotalFeesOverflow() (gas: 4604731)
Logs:
  starting total fees 800000000000000000
  ending total fees 153255926290448384

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 21.63ms (20.32ms CPU time)

Ran 1 test suite in 36.17ms (21.63ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

This works with different outputs!

[EngrPips]

Hello @sharonphiliplima, I am still trying to figure out the issue. Your test output result shows that your test was successful. Concerning the difference in the fee output, Patrick had a total of 93 participants by the end of the second raffle, and you had 95 total participants on a single raffle; the total difference in the total number of participants would affect the fee output. Also, why did you divide your expected fee by five after calculating it uint256 expectedFeeCollected = (entranceFee * 95) / 5;.

Finally, if the entrance fee charged is different, that would also affect the total fee calculation; I found out Patrick charges 1 Ether per entry from my calculation, so you should make sure your test is also charging the same to have the exact figure as Patrick.

[sharonphiliplima]

uint256 prizePool = (totalAmountCollected * 80) / 100;
        uint256 fee = (totalAmountCollected * 20) / 100;

This led me to divide it by 5.
maybe, I am missing something
as I had expected different values for my endingTotalFees and startingTotalFees!
Just like Patrick had 800000000000000000 and 153255926290448384 two different values

[EngrPips]

So i ralized that the issue arise from this line of your code;

console2.log("actualFeeCollected:", expectedFeeCollected);

You are passing the value of expectedFeeCollected while trying to console.log actualFeeCollected

[sharonphiliplima]

OMG I feel so silly rn

Ran 1 test for test/PuppyRaffleTest.t.sol:PuppyRaffleTest
[PASS] test_overflow_fees() (gas: 4797706)
Logs:
  expectedFeeCollected: 19000000000000000000
  actualFeeCollected: 553255926290448384

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 44.17ms (40.47ms CPU time)

Do I delete this q?

[EngrPips]

You don't need to delete it; it would make us realize that sometimes our problem is not as big as we think. Don't feel silly, as every single one of us has made this same mistake one way or the other