Cyfrin / foundry-smart-contract-lottery-cu

47 stars 38 forks source link

createSubscription function should support both Mock and Real VRFCoordinator contracts #57

Closed meitedaf closed 2 months ago

meitedaf commented 2 months ago

Issue Description:

The function is located in foundry-smart-contract-lottery-cu/script/Interactions.s.sol.

This implementation uses the VRFCoordinatorV2Mock type to call the createSubscription function, which means that regardless of whether the passed vrfCoordinator address is a mock or the actual VRFCoordinator contract, it will always use the mock contract's createSubscription function.

function createSubscription(
    address vrfCoordinator
) public returns (uint64) {
    console.log("Create subscription on ChainID: ", block.chainid);
    vm.startBroadcast();
    uint64 subId = VRFCoordinatorV2Mock(vrfCoordinator).createSubscription();
    vm.stopBroadcast();
    console.log("Your subId is: ", subId);
    console.log("Please update your subscriptionID in HelperConfig");
    return subId;
}
meitedaf commented 2 months ago

I watched the later video where you manually entered the subscriptionId πŸ˜‚, but before that I was confused, I thought I could create a real subscriptionId using the createSubscription function

cromewar commented 2 months ago

It's nice you pointed this out @meitedaf, it might be confusing for other users as well. Can you point us to the specific video on Updraft so we can add and update?

meitedaf commented 2 months ago

It's nice you pointed this out @meitedaf, it might be confusing for other users as well. Can you point us to the specific video on Updraft so we can add and update?

@cromewar, thank you for your response.

I have found the specific video on Updraft where the createSubscription function is discussed at the 5:23:00 mark. In the original implementation, regardless of whether the passed address is a mock or the actual VRFCoordinator contract, it always uses the mock contract's function. Therefore, I have modified the function to ensure that the mock contract is used on the local test network (chainid 31337) and the actual VRFCoordinator contract is used on other networks.

Here is my modified function:


function createSubscription(
    address vrfCoordinator
) public returns (uint64) {
    if (block.chainid == 31337) {
        console.log("Create subscription on ChainID: ", block.chainid);
        vm.startBroadcast();
        uint64 subId = VRFCoordinatorV2Mock(vrfCoordinator)
            .createSubscription();
        vm.stopBroadcast();
        console.log("Your subId is: ", subId);
        console.log("Please update your subscriptionID in HelperConfig");
        return subId;
    } else {
        vm.startBroadcast();
        uint64 subId = VRFCoordinatorV2(vrfCoordinator)
            .createSubscription();
        vm.stopBroadcast();
        return subId;
    }
}```

Video link: https://www.youtube.com/watch?v=sas02qSFZ74&t=4531s
meitedaf commented 2 months ago

By the way, I am currently watching the video at the provided link. At the 6:28:00 mark, the issue of the subscriptionId is discussed. Currently, it is not possible to create a Chainlink VRF v2 subscription via the VRF Subscription Manager, and thus, obtaining a subscriptionId is not feasible.

In my previously shared function, the else block:

else {
    vm.startBroadcast();
    uint64 subId = VRFCoordinatorV2(vrfCoordinator)
        .createSubscription();
    vm.stopBroadcast();
    return subId;
}

can create a new VRF v2 subscriptionId, thus allowing the current test to pass. However, I have noticed that the code in GitHub (https://github.com/Cyfrin/foundry-smart-contract-lottery-cu/blob/main/script/HelperConfig.s.sol) has been refactored to support VRF 2.5 subscriptions (where subscriptionId is of type uint256 instead of uint64 as in VRF 2).

Will this code refactoring be explained in the subsequent videos?

Thank you for your assistance!

PatrickAlphaC commented 2 months ago

@meitedaf sorry for the refactoring in the middle of you taking the course... We are going to fix that the next time we do a big release.

We made a lot of changes for the 2024 edition of Cyfrin Updraft, and these were some of them! So now, these should work now because we are using the same functions as the mock and real

uint256 subscriptionId = vrfCoordinatorV2_5Mock.createSubscription();

let me know if you'd like to discuss more.

meitedaf commented 2 months ago

@meitedaf sorry for the refactoring in the middle of you taking the course... We are going to fix that the next time we do a big release.

We made a lot of changes for the 2024 edition of Cyfrin Updraft, and these were some of them! So now, these should work now because we are using the same functions as the mock and real

uint256 subscriptionId = vrfCoordinatorV2_5Mock.createSubscription();

let me know if you'd like to discuss more.

@PatrickAlphaC, thank you for the clarification and for the updates in the 2024 edition of Cyfrin Updraft. The changes you mentioned will indeed help streamline the process by using the same functions for both mock and real subscriptions.

I have one more question: Will there be a video explaining the differences between the 2024 edition and the 2023 edition of Cyfrin Updraft? It would be very helpful to understand the specific changes and improvements made.

For those of us currently learning from the 2023 edition, what do you recommend? Should we switch to the corresponding sections in the 2024 edition or continue with the 2023 version? I'm very curious about when the 2024 edition will be released.

Thank you again for your support!

PatrickAlphaC commented 2 months ago

Ah... We'll need to address this better next year. I don't have plans to make such a video, to be honest, the biggest changes are this:

  1. We deploy a lot of our stuff to the ZKSync L2, and have additional ZKSync learnings
  2. New Section: Merkle Tree and Signatures by Ciara
  3. New Section: Account abstraction
  4. Completely re-filmed Smart Contract Lottery for v2.5 of Chainlink VRF
  5. Slight quality of life improvements

So most of it's the same with some tweaks. If you've been learning from the 2023 edition, you should be able to just keep going. Let me know if that's not the case.

The 2024 edition is already out on Cyfrin Updraft, and I'm attempting to upload it to YouTube now (but it's 38 hours so it might be a few more days, idk)

meitedaf commented 2 months ago

@PatrickAlphaC Thank you for the detailed response and for the updates in the 2024 edition of Cyfrin Updraft.

The updates you’ve mentioned sound very helpful, especially the deployment to ZKSync L2 and the new sections added. Even though there isn’t a dedicated video explaining the differences between the 2023 and 2024 editions, your summary is quite clear.

For those of us currently learning from the 2023 edition, your advice is very useful. I’ll continue with my current learning progress but will also keep an eye on the new content in the 2024 edition.

I’m looking forward to watching the 38-hour video once it’s uploaded to YouTube. If I encounter any issues during the learning process, I’ll reach out to you.

Thank you again for your support and hard work!

meitedaf commented 2 months ago

Hi @PatrickAlphaC, Unfortunately, I do have a problem that I can't solve. I am encountering an issue with the fulfillRandomWords function in the VRFCoordinatorV2Mock contract. The function call is reverting during my test, and I’m trying to understand the root cause.

Here is my test function testFulfillRandomWordsPicksAWinnerResetsAndSendMoney

function testFulfillRandomWordsPicksAWinnerResetsAndSendMoney()
        public
        enterRaffle
        timePassed
    {
        // Arrange
        uint256 startingIndex = 1;
        uint256 additionalEntrants = 5;
        uint256 previousTimestamp = raffle.getLastTimestamp();
        uint256 prize = entranceFee * (additionalEntrants + 1);
        for (
            uint256 i = startingIndex;
            i < startingIndex + additionalEntrants;
            i++
        ) {
            // address player = makeAddr("i");
            address player = address(uint160(i));
            // vm.deal(player, START_USER_BALANCE);
            // vm.prank(player);
            hoax(player, START_USER_BALANCE);
            raffle.enterRaffle{value: entranceFee}();
        }

        vm.recordLogs();
        raffle.performUpkeep(""); // emit requestId
        Vm.Log[] memory entries = vm.getRecordedLogs(); // capital V
        bytes32 requestId = entries[1].topics[1];
        console.log("Request ID: ", uint256(requestId));
        // pretend to be Chainlink VRF to get random number & pick winner
        try
            VRFCoordinatorV2Mock(vrfCoordinator).fulfillRandomWords(
                uint256(requestId),
                address(raffle)
            )
        {
            console.log("Fulfill Random Words succeeded");
        } catch Error(string memory reason) {
            console.log("Fulfill Random Words failed: ", reason);
        } catch (bytes memory reason) {
            console.logBytes(reason);
        }
        // Assert
        uint256 raffleState = uint256(raffle.getRaffleState());
        console.log("Raffle State: ", raffleState);
        address recentWinner = raffle.getRecentWinner();
        console.log("Recent Winner: ", recentWinner);
        uint256 lengthOfPlayers = raffle.getLengthOfPlayers();
        console.log("Length of Players: ", lengthOfPlayers);
        uint256 lastTimestamp = raffle.getLastTimestamp();
        console.log("Last Timestamp: ", lastTimestamp);

        assert(uint256(raffle.getRaffleState()) == 0);
        assert(raffle.getRecentWinner() != address(0));
        assert(raffle.getLengthOfPlayers() == 0);
        assert(previousTimestamp < raffle.getLastTimestamp());
        assert(
            address(raffle.getRecentWinner()).balance ==
                START_USER_BALANCE + prize - entranceFee
        );
        assert(address(raffle).balance == 0);
    }

In the VRFCoordinatorV2Mock contract, I’ve added logging to fulfillRandomWords to capture more details:

function fulfillRandomWords(uint256 _requestId, address _consumer) external nonReentrant {
        // console.log("Fulfilling Request ID: ", _requestId);
        // fulfillRandomWordsWithOverride(_requestId, _consumer, new uint256[](0));
        }

When running the test, the fulfillRandomWords function reverts with an error:

Logs:
  Funding subscription/subscriptionId is:  12033
  Using vrfCoordinator 0x8103B0A8A00be2DDC778e6e7eaa21791Cd364625
  On chainId:  11155111
  Using consumer contract:  0xBb2180ebd78ce97360503434eD37fcf4a1Df61c3
  Using vrfCoordinator 0x8103B0A8A00be2DDC778e6e7eaa21791Cd364625
  On chainId:  11155111
  Request ID:  45767679224649581453707126129566934396995937542817770060634713150277956150036
  0x
  Raffle State:  1
  Recent Winner:  0x0000000000000000000000000000000000000000
  Length of Players:  6
  Last Timestamp:  1720963884
  ......
  0] console::log("Request ID: ", 45767679224649581453707126129566934396995937542817770060634713150277956150036 [4.576e76]) [staticcall]
    β”‚   └─ ← [Stop] 
    β”œβ”€ [237] 0x8103B0A8A00be2DDC778e6e7eaa21791Cd364625::fulfillRandomWords(1623701109742839672485506587998590406944772821880539981 [1.623e54], Raffle: [0xBb2180ebd78ce97360503434eD37fcf4a1Df61c3])
    β”‚   └─ ← [Revert] EvmError: Revert
    β”œβ”€ [0] console::logBytes(0x) [staticcall]
    β”‚   └─ ← [Stop] 
    β”œβ”€ [378] Raffle::getRaffleState() [staticcall]
    β”‚   └─ ← [Return] 1
    β”œβ”€ [0] console::log("Raffle State: ", 1) [staticcall]
    β”‚   └─ ← [Stop] 
    β”œβ”€ [2364] Raffle::getRecentWinner() [staticcall]
    β”‚   └─ ← [Return] 0x0000000000000000000000000000000000000000
    β”œβ”€ [0] console::log("Recent Winner: ", 0x0000000000000000000000000000000000000000) [staticcall]
    β”‚   └─ ← [Stop] 
    β”œβ”€ [304] Raffle::getLengthOfPlayers() [staticcall]
    β”‚   └─ ← [Return] 6
    β”œβ”€ [0] console::log("Length of Players: ", 6) [staticcall]
    β”‚   └─ ← [Stop] 
    β”œβ”€ [281] Raffle::getLastTimestamp() [staticcall]
    β”‚   └─ ← [Return] 1720963884 [1.72e9]
    β”œβ”€ [0] console::log("Last Timestamp: ", 1720963884 [1.72e9]) [staticcall]
    β”‚   └─ ← [Stop] 
    β”œβ”€ [378] Raffle::getRaffleState() [staticcall]
    β”‚   └─ ← [Return] 1
    └─ ← [Revert] panic: assertion failed (0x01)

Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 16.53s (3.58s CPU time)

Ran 1 test suite in 20.00s (16.53s CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in test/unit/RaffleTest.t.sol:RaffleTest
[FAIL. Reason: panic: assertion failed (0x01)] testFulfillRandomWordsPicksAWinnerResetsAndSendMoney() (gas: 319560)

The generated requestId seems to be invalid, as indicated by the revert in the fulfillRandomWords function. I've tried a lot of methods, adding console.log to the first line of fulfillRandomWords function, and it doesn't execute, meaning that just as soon as fulfillRandomWords is called, revert happens.

This issue might be quite complicated, especially without specific context. If it proves to be very tricky, please let me know and don't spend too much time on it. I understand you're very busy and have already made a lot of instructional videos for us and helped us solve problems. I'll put aside the 23 version and watch the 24 version videos instead.