Broke something in fundmetest.s.sol when creating testWithDrawFromMultipleFundersCheaper

[SquilliamX]

I was following the course in Lesson 7 and around 1:53:26 Patrick says to copy testWithDrawFromMultipleFunders and paste it and convert it to testWithDrawFromMultipleFundersCheaper. Then within testWithDrawFromMultipleFundersCheaper only change fundMe.withdraw to fundMe.cheaperWithdraw. Then run forge snapshot. Unfortunately, even in this simple step, I managed to break something :( .

Here is my error code when i run forge snapshot -

~/code/foundry-f23/foundry-fund-me-f23$ forge snapshot
[⠰] Compiling...
[⠆] Compiling 3 files with 0.8.20
[⠰] Solc 0.8.20 finished in 1.82s
Compiler run successful!

Running 10 tests for test/FundMeTest.t.sol:FundMeTest
[PASS] testAddsFunderToArrayOfFunders() (gas: 99670)
[PASS] testFundFailsWithoutEnoughETH() (gas: 22968)
[FAIL. Reason: Assertion failed.] testFundUpdatesFundedDataStructure() (gas: 113554)
[PASS] testMinimumDollarIsFive() (gas: 5405)
[PASS] testOnlyOwnerCanWithdraw() (gas: 99977)
[PASS] testOwnerIsMsgSender() (gas: 5565)
[PASS] testPriceFeedVersionIsAccurate() (gas: 10692)
[PASS] testWithDrawFromMultipleFunders() (gas: 488240)
[FAIL. Reason: Assertion violated] testWithDrawFromMultipleFundersCheaper() (gas: 606665)
[PASS] testWithDrawWithASingleFunder() (gas: 87345)
Test result: FAILED. 8 passed; 2 failed; finished in 9.41ms

Failing tests:
Encountered 2 failing tests in test/FundMeTest.t.sol:FundMeTest
[FAIL. Reason: Assertion failed.] testFundUpdatesFundedDataStructure() (gas: 113554)
[FAIL. Reason: Assertion violated] testWithDrawFromMultipleFundersCheaper() (gas: 606665)

i tried to narrow down the problem but i dont understand what the problem is here-

~/code/foundry-f23/foundry-fund-me-f23$ forge test --match-test testWithDrawFromMultipleFundersCheaper -vvv
[⠰] Compiling...
No files changed, compilation skipped

Running 1 test for test/FundMeTest.t.sol:FundMeTest
[FAIL. Reason: Assertion violated] testWithDrawFromMultipleFundersCheaper() (gas: 606665)
Traces:
  [485332] FundMeTest::testWithDrawFromMultipleFundersCheaper() 
    ├─ [0] VM::prank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) 
    │   └─ ← ()
    ├─ [81357] FundMe::fund{value: 100000000000000000}() 
    │   ├─ [8993] MockV3Aggregator::latestRoundData() [staticcall]
    │   │   └─ ← 1, 200000000000, 1, 1, 1
    │   └─ ← ()
    ├─ [0] VM::deal(0x0000000000000000000000000000000000000001, 100000000000000000 [1e17]) 
    │   └─ ← ()
    ├─ [0] VM::prank(0x0000000000000000000000000000000000000001) 
    │   └─ ← ()
    ├─ [46957] FundMe::fund{value: 100000000000000000}() 
    │   ├─ [993] MockV3Aggregator::latestRoundData() [staticcall]
    │   │   └─ ← 1, 200000000000, 1, 1, 1
    │   └─ ← ()
    ├─ [0] VM::deal(0x0000000000000000000000000000000000000002, 100000000000000000 [1e17]) 
    │   └─ ← ()
    ├─ [0] VM::prank(0x0000000000000000000000000000000000000002) 
    │   └─ ← ()
    ├─ [46957] FundMe::fund{value: 100000000000000000}() 
    │   ├─ [993] MockV3Aggregator::latestRoundData() [staticcall]
    │   │   └─ ← 1, 200000000000, 1, 1, 1
    │   └─ ← ()
    ├─ [0] VM::deal(0x0000000000000000000000000000000000000003, 100000000000000000 [1e17]) 
    │   └─ ← ()
    ├─ [0] VM::prank(0x0000000000000000000000000000000000000003) 
    │   └─ ← ()
    ├─ [46957] FundMe::fund{value: 100000000000000000}() 
    │   ├─ [993] MockV3Aggregator::latestRoundData() [staticcall]
    │   │   └─ ← 1, 200000000000, 1, 1, 1
    │   └─ ← ()
    ├─ [0] VM::deal(0x0000000000000000000000000000000000000004, 100000000000000000 [1e17]) 
    │   └─ ← ()
    ├─ [0] VM::prank(0x0000000000000000000000000000000000000004) 
    │   └─ ← ()
    ├─ [46957] FundMe::fund{value: 100000000000000000}() 
    │   ├─ [993] MockV3Aggregator::latestRoundData() [staticcall]
    │   │   └─ ← 1, 200000000000, 1, 1, 1
    │   └─ ← ()
    ├─ [0] VM::deal(0x0000000000000000000000000000000000000005, 100000000000000000 [1e17]) 
    │   └─ ← ()
    ├─ [0] VM::prank(0x0000000000000000000000000000000000000005) 
    │   └─ ← ()
    ├─ [46957] FundMe::fund{value: 100000000000000000}() 
    │   ├─ [993] MockV3Aggregator::latestRoundData() [staticcall]
    │   │   └─ ← 1, 200000000000, 1, 1, 1
    │   └─ ← ()
    ├─ [0] VM::deal(0x0000000000000000000000000000000000000006, 100000000000000000 [1e17]) 
    │   └─ ← ()
    ├─ [0] VM::prank(0x0000000000000000000000000000000000000006) 
    │   └─ ← ()
    ├─ [46957] FundMe::fund{value: 100000000000000000}() 
    │   ├─ [993] MockV3Aggregator::latestRoundData() [staticcall]
    │   │   └─ ← 1, 200000000000, 1, 1, 1
    │   └─ ← ()
    ├─ [0] VM::deal(0x0000000000000000000000000000000000000007, 100000000000000000 [1e17]) 
    │   └─ ← ()
    ├─ [0] VM::prank(0x0000000000000000000000000000000000000007) 
    │   └─ ← ()
    ├─ [46957] FundMe::fund{value: 100000000000000000}() 
    │   ├─ [993] MockV3Aggregator::latestRoundData() [staticcall]
    │   │   └─ ← 1, 200000000000, 1, 1, 1
    │   └─ ← ()
    ├─ [0] VM::deal(0x0000000000000000000000000000000000000008, 100000000000000000 [1e17]) 
    │   └─ ← ()
    ├─ [0] VM::prank(0x0000000000000000000000000000000000000008) 
    │   └─ ← ()
    ├─ [46957] FundMe::fund{value: 100000000000000000}() 
    │   ├─ [993] MockV3Aggregator::latestRoundData() [staticcall]
    │   │   └─ ← 1, 200000000000, 1, 1, 1
    │   └─ ← ()
    ├─ [0] VM::deal(0x0000000000000000000000000000000000000009, 100000000000000000 [1e17]) 
    │   └─ ← ()
    ├─ [0] VM::prank(0x0000000000000000000000000000000000000009) 
    │   └─ ← ()
    ├─ [46957] FundMe::fund{value: 100000000000000000}() 
    │   ├─ [993] MockV3Aggregator::latestRoundData() [staticcall]
    │   │   └─ ← 1, 200000000000, 1, 1, 1
    │   └─ ← ()
    ├─ [235] FundMe::getOwner() [staticcall]
    │   └─ ← DefaultSender: [0x1804c8AB1F12E6bbf3894d4083f33e07309d1f38]
    ├─ [235] FundMe::getOwner() [staticcall]
    │   └─ ← DefaultSender: [0x1804c8AB1F12E6bbf3894d4083f33e07309d1f38]
    ├─ [0] VM::startPrank(DefaultSender: [0x1804c8AB1F12E6bbf3894d4083f33e07309d1f38]) 
    │   └─ ← ()
    ├─ [6597] FundMe::cheaperWithdraw() 
    │   └─ ← ()
    ├─ [0] VM::stopPrank() 
    │   └─ ← ()
    └─ ← "Assertion violated"

Test result: FAILED. 0 passed; 1 failed; finished in 7.20ms

Failing tests:
Encountered 1 failing test in test/FundMeTest.t.sol:FundMeTest
[FAIL. Reason: Assertion violated] testWithDrawFromMultipleFundersCheaper() (gas: 606665)

Encountered a total of 1 failing tests, 0 tests succeeded

here is fundme.sol-

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.18;

import {AggregatorV3Interface} from "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
import {PriceConverter} from "./PriceConverter.sol";

error FundMe__NotOwner();

contract FundMe {
    using PriceConverter for uint256;

    mapping(address => uint256) private s_addressToAmountFunded;
    address[] private s_funders;

    // Could we make this constant?  /* hint: no! We should make it immutable! */
    address private immutable i_owner;
    uint256 public constant MINIMUM_USD = 5 * 10 ** 18;
    AggregatorV3Interface private s_priceFeed;

    constructor(address priceFeed) {
        i_owner = msg.sender;
        s_priceFeed = AggregatorV3Interface(priceFeed);
    }

    function fund() public payable {
        require(
            msg.value.getConversionRate(s_priceFeed) >= MINIMUM_USD,
            "You need to spend more ETH!"
        );
        // require(PriceConverter.getConversionRate(msg.value) >= MINIMUM_USD, "You need to spend more ETH!");
        s_addressToAmountFunded[msg.sender] += msg.value;
        s_funders.push(msg.sender);
    }

    function getVersion() public view returns (uint256) {
        return s_priceFeed.version();
    }

    modifier onlyOwner() {
        // require(msg.sender == owner);
        if (msg.sender != i_owner) revert FundMe__NotOwner();
        _;
    }

    function cheaperWithdraw() public onlyOwner {
        uint256 fundersLength = s_funders.length;
        for (
            uint256 funderIndex = 0;
            funderIndex < fundersLength;
            funderIndex++
        ) {
            address funder = s_funders[funderIndex];
            s_addressToAmountFunded[funder] = 0;
        }
        s_funders = new address[](0);
    }

    function withdraw() public onlyOwner {
        for (
            uint256 funderIndex = 0;
            funderIndex < s_funders.length;
            funderIndex++
        ) {
            address funder = s_funders[funderIndex];
            s_addressToAmountFunded[funder] = 0;
        }
        s_funders = new address[](0);
        // // transfer
        // payable(msg.sender).transfer(address(this).balance);

        // // send
        // bool sendSuccess = payable(msg.sender).send(address(this).balance);
        // require(sendSuccess, "Send failed");

        // call
        (bool callSuccess, ) = payable(msg.sender).call{
            value: address(this).balance
        }("");
        require(callSuccess, "Call failed");
    }

    // Explainer from: https://solidity-by-example.org/fallback/
    // Ether is sent to contract
    //      is msg.data empty?
    //          /   \
    //         yes  no
    //         /     \
    //    receive()?  fallback()
    //     /   \
    //   yes   no
    //  /        \
    //receive()  fallback()

    fallback() external payable {
        fund();
    }

    receive() external payable {
        fund();
    }

    /**
     * View / Pure functions (Getters)
     */

    function getAddressToAmountFunded(
        address fundingAddress
    ) external view returns (uint256) {
        return s_addressToAmountFunded[fundingAddress];
    }

    function getFunders(uint256 index) external view returns (address) {
        return s_funders[index];
    }

    function getOwner() external view returns (address) {
        return i_owner;
    }
}

// Concepts we didn't cover yet (will cover in later sections)
// 1. Enum
// 2. Events
// 3. Try / Catch
// 4. Function Selector
// 5. abi.encode / decode
// 6. Hash with keccak256
// 7. Yul / Assembly

here is fundmetest.s.sol-

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.18;

import {Test, console} from "forge-std/Test.sol";
import {FundMe} from "src/FundMe.sol";
import {DeployFundMe} from "../script/DeployFundMe.s.sol";

contract FundMeTest is Test {
    FundMe fundMe;

    address USER = makeAddr("user");
    uint256 constant SEND_VALUE = 0.1 ether; // 100000000000000000
    uint256 constant STARTING_BALANCE = 10 ether;
    uint256 constant GAS_PRICE = 1;

    function setUp() external {
        // fundMe = new FundMe(0x694AA1769357215DE4FAC081bf1f309aDC325306);
        DeployFundMe deployFundMe = new DeployFundMe();
        fundMe = deployFundMe.run();
        vm.deal(USER, STARTING_BALANCE);
    }

    function testMinimumDollarIsFive() public {
        assertEq(fundMe.MINIMUM_USD(), 5e18);
    }

    function testOwnerIsMsgSender() public {
        assertEq(fundMe.getOwner(), msg.sender);
    }

    function testPriceFeedVersionIsAccurate() public {
        uint256 verison = fundMe.getVersion();
        assertEq(verison, 4);
    }

    function testFundFailsWithoutEnoughETH() public {
        vm.expectRevert(); // hey, the next line should revert
        // assert(this tx fails/reverts)
        fundMe.fund(); // send 0 value
    }

    function testFundUpdatesFundedDataStructure() public {
        vm.prank(USER); // The next transaction will be sent by USER
        fundMe.fund{value: SEND_VALUE}();
        uint256 amountFunded = fundMe.getAddressToAmountFunded(address(this));
        assertEq(amountFunded, SEND_VALUE);
    }

    function testAddsFunderToArrayOfFunders() public {
        vm.prank(USER); // The next transaction will be sent by USER
        fundMe.fund{value: SEND_VALUE}();

        address funder = fundMe.getFunders(0);
        assertEq(funder, USER);
    }

    modifier funded() {
        vm.prank(USER); // The next transaction will be sent by USER
        fundMe.fund{value: SEND_VALUE}();
        _;
    }

    function testOnlyOwnerCanWithdraw() public funded {
        vm.prank(USER);
        vm.expectRevert();
        fundMe.withdraw();
    }

    function testWithDrawWithASingleFunder() public funded {
        // Arrange
        uint256 startingOwnerBalance = fundMe.getOwner().balance;
        uint256 startingFundMeBalance = address(fundMe).balance;
        // Act
        uint256 gasStart = gasleft(); //1000 gas
        vm.txGasPrice(GAS_PRICE);
        vm.prank(fundMe.getOwner()); // c: 200 gas
        fundMe.withdraw();

        uint256 gasEnd = gasleft(); // 800 gas
        uint256 gasUsed = (gasStart - gasEnd) * tx.gasprice; // 200 gas
        console.log(gasUsed);

        // Assert
        uint256 endingOwnerBalance = fundMe.getOwner().balance;
        uint256 endingFundMeBalance = address(fundMe).balance;
        assertEq(endingFundMeBalance, 0);
        assertEq(
            startingFundMeBalance + startingOwnerBalance,
            endingOwnerBalance
        );
    }

    function testWithDrawFromMultipleFunders() public funded {
        // Arrange
        uint160 numberOfFunders = 10;
        uint160 startingFunderIndex = 1;
        for (uint160 i = startingFunderIndex; i < numberOfFunders; i++) {
            // vm.prank new address
            // vm.deal new address
            // address(0)
            hoax(address(i), SEND_VALUE);
            fundMe.fund{value: SEND_VALUE}();
        }

        uint256 startingOwnerBalance = fundMe.getOwner().balance;
        uint256 startingFundMeBalance = address(fundMe).balance;

        // Act
        vm.startPrank(fundMe.getOwner());
        fundMe.withdraw();
        vm.stopPrank();

        // Assert
        assert(address(fundMe).balance == 0);
        assert(
            startingFundMeBalance + startingOwnerBalance ==
                fundMe.getOwner().balance
        );
    }

    function testWithDrawFromMultipleFundersCheaper() public funded {
        // Arrange
        uint160 numberOfFunders = 10;
        uint160 startingFunderIndex = 1;
        for (uint160 i = startingFunderIndex; i < numberOfFunders; i++) {
            // vm.prank new address
            // vm.deal new address
            // address(0)
            hoax(address(i), SEND_VALUE);
            fundMe.fund{value: SEND_VALUE}();
        }

        uint256 startingOwnerBalance = fundMe.getOwner().balance;
        uint256 startingFundMeBalance = address(fundMe).balance;

        // Act
        vm.startPrank(fundMe.getOwner());
        fundMe.cheaperWithdraw();
        vm.stopPrank();

        // Assert
        assert(address(fundMe).balance == 0);
        assert(
            startingFundMeBalance + startingOwnerBalance ==
                fundMe.getOwner().balance
        );
    }
}

Can anyone help me solve this problem on why I have 2 failing tests? thanks in advance!

[Abelaby]

Your testFundUpdatesFundeddataStructure() is calling the getAddressToAmountFunded function with the wrong address it should be as follows:

        uint256 amountFunded = fundMe.getAddressToAmountFunded(USER);

As for why the testWithDrawFromMultipleFundersCheaper() is failing, upon tracing back to you cheaperWithdraw() function , the call is missing and it should be okay once you add the following line:

(bool callSuccess, ) = payable(msg.sender).call{value: address(this).balance}("");
        require(callSuccess, "Call failed");

Similar to our previous withdraw() function.
Hope this helps.

[SquilliamX]

It worked! thank you abelaby!