Cyfrin / foundry-full-course-cu

GNU General Public License v3.0
3.7k stars 916 forks source link

`testPriceFeedVersionIsAccurate` should be updated after we refactored with the helper config #2895

Closed SquilliamX closed 4 days ago

SquilliamX commented 4 days ago

Section

Section 7 | Foundry Fund Me

Could you please leave a link to the Cyfrin Updraft Lesson (or YouTube video timestamp) where this error occurs? (You can right click a video and "copy video URL at current time")

I can't copy the video url but its from 12:00 to 12:11, after we finished refactoring.

Describe the bug

In episode 11 of Foundry FundMe, "Deploy a Mock PriceFeed" @ 12:11 , we run forge test --fork -url $MAINNET_RPC_URL and all the tests pass in the video but not for users following. This is outdated as this test function is asserting that the mainnet pricefeed version == 4. This is no longer correct. Chainlink updated the pricefeed verisons, so while the eth/usd sepolia pricefeed version is still 4, Chainlink's eth/usd mainnet priceFeed version is now 6. So the test testPriceFeedVersionIsAccurate should be changed to the following:

  function testPriceFeedVersionIsAccurate() public {
        if (block.chainid == 11155111) {
            uint256 version = fundMe.getVersion();
            assertEq(version, 4);
        } else if (block.chainid == 1) {
            uint256 version = fundMe.getVersion();
            assertEq(version, 6);
        }
  }       

This way if you run forge test --mt testPriceFeedVersionIsAccurate -vvvv --fork-url $SEPOLIA_RPC_URL or forge test --mt testPriceFeedVersionIsAccurate -vvvv --fork-url $MAINNET_RPC_URL the test will always pass.

EngrPips commented 4 days ago

Hey @SquilliamX, Seem like you figured this one out.

SquilliamX commented 4 days ago

Hey @SquilliamX, Seem like you figured this one out.

Hey I closed this because i opened this issue as a Video Mistake instead of Code Mistake so i closed this and created one under Code Mistake. Its a code mistake that is outdated and should be fixed to help new beginners.

EngrPips commented 3 days ago

Oh, I see, Thanks for the contribution.