Skip to content
This repository was archived by the owner on Aug 8, 2024. It is now read-only.

Converting the ERC721 exercise to Cairo V1. #26

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

robertkodra
Copy link

This is the first commit to transition the ERC721 to Cairo V1.

I've divided the exercises into two parts. The first part is building your ERC721 from scratch. The second part is still WIP, which focuses on adding the exercises that are in the Cairo V0 repo, mainly creating features such as wings, legs, etc and the rest of the exercises.

Regarding Part 1 I've created the following:

  • The Evaluator file, which will check for 4 exercises:
  1. Checks if the user initialized the ERC721 with the correct name and symbol. The name and symbol is assigned randomly by the Evaluator by getting a user_slot number from the Evaluator.
  2. Checks if the user created the mint function correctly and it will attempt to mint a token to itself.
  3. Checks if the burn function has been implemented correctly and will need to have a token before the function can start.
  4. Checks if the transfer_from() function transfer has bee implemented correctly.

I've made the PR in draft as there are still things to work out on the first part.

@omarespejel omarespejel self-requested a review May 24, 2023 18:18
@omarespejel
Copy link

Hey @robertkodra,

Firstly, my apologies for the delay in responding to your PR; the lapse was entirely on my end. I will give priority to reviewing the conversion of this workshop.

I've taken the time to review your work on transitioning the ERC721 to Cairo V1, and I must say, I'm genuinely impressed. You've shown a clear understanding of the task at hand and your work is in line with our prior discussions.

  • The two-part structure of the exercises is well-thought-out and organized. It should provide learners with a thorough understanding of building an ERC721 from scratch and subsequently adding additional features. The Evaluator file is also a crutial addition.
  • Your checks for the mint, burn, and transfer_from() functions are quite essential and should ensure users' correct implementation of these functions.
  • For the work-in-progress section, I'd suggest considering the inclusion of the approve() and setApprovalForAll() functions as additional exercises. These functions are also part of the standard ERC721 functionality and might serve as valuable learning points.

Once again, kudos on your fantastic work so far. I look forward to seeing your progress on the ERC721 workshop.

@robertkodra
Copy link
Author

Thanks @omarespejel for the review. I will try to submit a few updates this weekend and add the rest of the contract to it. I'll tag you once I make the changes.

@omarespejel
Copy link

Thank you @robertkodra! Feel free to ask me whatever you need. Thanks!

@robertkodra robertkodra marked this pull request as ready for review June 18, 2023 16:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants