[ Lesson 13: Hardhat DeFi & Aave ] - mintNft() function

[MrAbuz]

Hello guys! Hope everyone's well, and doing well in the course!
I'm having a super basic question here. In the Lesson 13 -> DynamicSvgNft.sol, we have:

    function mintNft(int256 highValue) public {
        s_tokenIdToHighValues[s_tokenCounter] = highValue;
        _safeMint(msg.sender, s_tokenCounter);
        s_tokenCounter = s_tokenCounter + 1;
        emit CreatedNFT(s_tokenCounter, highValue);
    }

If we do "s_tokenCounter + 1" between minting and emitting, won't the emit send a different s_tokenCounter than the one that was used in the mint? The purpose should be to emit the same tokenCounter (which is the nft tokenId) that was minted.

I got even more confused because in the video (23:14:23) Patrick suggested that "it's a best practise to update the token counter before actually doing the minting". So in the video he did it like this:

    function mintNft(int256 highValue) public {
        s_tokenIdToHighValues[s_tokenCounter] = highValue;
        s_tokenCounter = s_tokenCounter + 1;
        _safeMint(msg.sender, s_tokenCounter);
        emit CreatedNFT(s_tokenCounter, highValue);
    }

But like this, in the first line, the user will update the s_tokenIdToHighValues mapping using a different token counter than the token counter that will be assigned to him in the mint. Which, if someone remembers this NFT project, will make the user assign an ETH price for his NFT to change face to a different token ID than the one that will be assigned to him in the mint, which would make him not being able to choose the ETH price for his NFT.

Should the code be as following (updating token counter before anything else), or what am I missing?

    function mintNft(int256 highValue) public {
        s_tokenCounter = s_tokenCounter + 1;
        s_tokenIdToHighValues[s_tokenCounter] = highValue;
        _safeMint(msg.sender, s_tokenCounter);
        emit CreatedNFT(s_tokenCounter, highValue);
    }

Thank you in advance!! eheh

[DorianDaSilva]

@MrAbuz

The token counter + 1 is used to keep track of NFTs minted.

For example, I mint an NFT token 001 then you mint the same one. Yours will be token counter + 1 or NFT token 002 that way each of our NFT ownership is clearly defined.

I hope this helps answering your question

[MrAbuz]

Thanks for the help @DorianDaSilva ! :)

[alymurtazamemon]

@MrAbuz Yup, your curiosity is right. Here is the way we update the token counter;

uint256 private s_tokenCounter;

constructor() {
    s_tokenCounter = 0;
}

function mintNft(int256 highValue) public {
    uint256 tokenId = s_tokenCounter;
    s_tokenCounter = s_tokenCounter + 1;
    // ...do further things
}

Now you use the same tokenId for the minting and for the event as well. In addition to that, you also update the state of a counter before making the external calls.

There is the issue in the repo code and due to that people face issues.

[MrAbuz]

Super thank you @alymurtazamemon ! That's smart!

This way we can have the first NFT be token Id = 0, while making the rest work properly! Because with my solution I guess it would all work the same but the first minted nft would start at token id = 1!

By curiosity, what do you mean with updating the state of the counter before making external calls? Which external call are we doing, and why is it important to update the state before doing it?

Thank you a lot again! :D

[alymurtazamemon]

@MrAbuz In this example we do not have external calls, external calls are like calling other contracts using address.call{value: any(""); and in order to be safe from reentrancy attacks we should practice updating the start before calling those. Other solution is to use reentrancy guards from openzeppelin or create your own one.

[MrAbuz]

Okok! I was confused as reentrancy attacks from my knowledge required us calling other contracts, and we weren't doing that here!

Thank you for your help, super appreciated! ;)