DeployRaffle script returns incorrect HelperConfig type - propagating errors to RaffleTest setUp() function.

[simon-masterclass]

Section

Section 9 | Smart Contract Lottery

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")

https://updraft.cyfrin.io/courses/foundry/smart-contract-lottery/setup-solidity-lottery-tests?lesson_format=video

Operating System

macOS (Apple Silicon)

Describe the bug

Hi @PatrickAlphaC & team, I really enjoy your courses - I've learned a TON. Correct me if I'm wrong but it appears that the DeployRaffle script returns the wrong HelperConfig type. Also, the way HelperConfig is initiated in the test setUp() function also appears incorrect as a result. Here's a link to the code in question: https://github.com/Cyfrin/foundry-smart-contract-lottery-cu/blob/main/script/DeployRaffle.s.sol

The specific issue is on lines number 10 and 39. Here's a snippet of line 10:

contract DeployRaffle is Script {
    function run() external returns (Raffle, HelperConfig) {
        HelperConfig helperConfig = new HelperConfig(); // This comes with our mocks!
        AddConsumer addConsumer = new AddConsumer();
        HelperConfig.NetworkConfig memory config = helperConfig.getConfig();

if (config.subscriptionId == 0) {
            CreateSubscription createSubscription = new CreateSubscription();
            (config.subscriptionId, config.vrfCoordinatorV2_5) =
                createSubscription.createSubscription(config.vrfCoordinatorV2_5, config.account);

            FundSubscription fundSubscription = new FundSubscription();
            fundSubscription.fundSubscription(
                config.vrfCoordinatorV2_5, config.subscriptionId, config.link, config.account
            );
        }

So, because we are creating an in-memory instance of HelperConfig named "config" and then modifying it in the "if" statement (sometimes), the return type shouldn't be "HelperConfig" but "HelperConfig.NetworkConfig memory" instead.

That way, we can return, the "config" at the end instead of an instance of HelpConfig, which doesn't have an updated version of the config.subscriptionId - as we see on line 39:

return (raffle, helperConfig);

Also, the setUp() function in the RaffleTest contract will need to be modified to something more like this.

function setUp() public {
        // ARRANGE
        // Setup: Deploy Raffle contract
        DeployRaffle deployRaffle = new DeployRaffle();
        // Setup: Get the Raffle contract and HelperConfig settings
        HelperConfig.NetworkConfig memory config;
        (raffle, config) = deployRaffle.deployContract();

Let me know if I missed anything. Thanks! -Simon

[EngrPips]

Hello @simon-masterclass, Thanks for pointing this out. It will be attended to as soon as possible.

[ciaranightingale]

Hi @simon-masterclass I have verified that this does look to be an issue by running the tests on a Speolia fork and verifying that the ID is not updated, still returns 0 and the tests no longer pass - thanks for pointing this out, I have escalated this and we are attending to this now:)

[PatrickAlphaC]

Great callout! We did forget to set it! It was working for me because my fork tests were running off my subscription which I had set!

This is why code reviews are so important in web3 :)

Thank you! This has been fixed here:

https://github.com/Cyfrin/foundry-smart-contract-lottery-cu/commit/51639f2898bbaaa8d055f9f1af27c02272d0d5a6

We added a setter to the deploy function.