Final Project Feedback

[fodisi]

Hi @Phasor , I'm a mentor of the consensy bootcamp responsible for grading your project. Congratulations, your Final Project passed! We'll be approving it on the course itself in a week or two, but here's your feedback in the meanwhile:

Front-End

• Nice work putting together the FE to mint NFTs and detecting account changes.
• Good to see you deployed your contractst to rinkeby. Did you use truffle migrate for that or something else? Based on the readme, I'd assume it was truffle - just just wondering :D
• great job deploying the front-end to netlify. Did you have previous experience with it? If not, how was the process (difficult, easy)?

Unit Testing

• Nice to see you implemented the unit tests to check for basic functionality of the contract. Keep on improving and adding more tests to cover more scenarios and functions. The closest to 100% coverage the better.
• Good coverage of the basic functionalities of the contract.As a next step, I'd suggest you also add unit tests to make sure your modifiers/require statements work as expected. This means adding unit tests to make sure a transaction reverts when it is supposed to revert (as you did in the test "should NOT let non-owners change the tokenURI"). An example is making sure a tx reverts if the user tries to mint more tokens than the "maxMintAmount"
• You could avoid duplicating the logic of always getting the deployed instance of the nftInstance token by leveraging on the "beforeEach" feature of the testing framework.

Smart Contract

• Contract is simple and concise, with good readability.
• Nice to see you implementing a ""Pausable"" functionality in the contract. Wondering why did you decide to implement it on your own instead of also using OpenZeppelin's ""Pausable"" contract (considering that you already use OpenZeppelin for other functionalities). Is there something specific you'd want to achieve?
• Good choice with using OpenZeppelin's contracts for access control (Ownable) and ERC721Enumerable, and properly adding the onlyOwner modifier when applicable.
• Great to see you using the Checks-Effects-Interactions pattern in the mint function to avoid reentrancy.
• I noted you control the number of NFTs a user can mint per transaction. However, there's no control to avoid that the same user mints more than ""maxMintAmount"" if the user calls the mint function multiple times. Is that the desired behavior or something that still needs to be worked on?
• Great that you used OpenZeppelin library to control ownability and to expand the logic of your NFTs. Always good to use battle-tested libraries ;-)"

General Feedback

• It'd be great to see the FE detecting the network the user is connected to, and alerting/prompting the user to switch to the correct one ;-)
• Nice project and nice work putting all the pieces together. Kudos to using React/hooks on your FE and exploring with OpenZeppeling library."

[Phasor]

Thank you Ser! Getting feedback is so useful.

1. Yes I used truffle to deploy to Rinkeby
2. I used Netlify once before yes, it's pretty easy to use. Recommended
3. I was not actually aware of OpenZeppelin's pausable contract, I will take a look thanks
4. I am actually not certain if I want a hard cap on mints, I think I need to think about that and work on it in the future
5. I will add the auto detect of the network, thank you for the suggestion

Thank you for taking the time to go through it so thoroughly!