StarknetByExample feat: added an ERC721 NFT Contract

Issue(s): https://github.com/NethermindEth/StarknetByExample/issues/197

Description

Implemented a simple ERC721 Non-Fungible Token (NFT) contract

Checklist

[x] Contract Tests: Added tests to cover the changes

Jun 04 '24 22:06 raizo07

You need to add a md page in src, detailing each methods of the interface as well.

Alright I'll submit a PR for that along side the test today.

Jun 05 '24 07:06 raizo07

@raizo07 need any help with this?

Jun 07 '24 08:06 0xNeshi

@raizo07 need any help with this?

I'm on it already, when I'm done I'll make it a draft PR so you can help review before the main review if that's okay with you.

Jun 07 '24 08:06 raizo07

@julio4 Kindly review this.

Jun 18 '24 03:06 raizo07

Hello @julio4 @misicnenad Any chance of you reviewing this soon?

Jun 20 '24 12:06 raizo07

A couple of things:

1. You forgot to address [Jules change request related to `mod errors`](https://github.com/NethermindEth/StarknetByExample/pull/214/files#r1637947087).

2. You are missing [Scarb.toml](https://book.cairo-lang.org/ch13-00-introduction-to-starknet-smart-contracts.html?highlight=scarb.toml#scarb) file.

3. Expanding on the previous point, you should use a valid Cairo project structure, e.g. see [listings/templates/scarb](https://github.com/NethermindEth/StarknetByExample/tree/main/listings/templates/scarb).
   You can easily create the correct Cairo project structure using the `scarb new <project-name>` command (see [The Cairo Book](https://book.cairo-lang.org/ch01-02-hello-world.html#creating-a-project-with-scarb) for more details).

4. The `IERC721` interface in the PR does not match the conventional one:
   
   * different function names
   * should use `u256` instead of felts and `u128`, as [Jules said at the end of his original comment](https://github.com/NethermindEth/StarknetByExample/pull/214#discussion_r1640884954)
   * `IERC721` does not contain functions to get `name`, `symbol` and `token_uri` (those are part of `IERC721Metadata`).

5. To confirm you implemented everything correctly, you could run `bash scripts/cairo_programs_verifier.sh` from the root directory - it will pinpoint any potential issues in your code (see project's [Contributing guidelines](https://github.com/NethermindEth/StarknetByExample/blob/main/CONTRIBUTING.md#verification-script)).

You can use OpenZeppelin's ERC721 implementation as inspiration for this PR, it will make it much easier to implement, see https://github.com/OpenZeppelin/cairo-contracts/blob/main/src/token/erc721.

Yes, this are all valid concerns outlined by @misicnenad

Jun 21 '24 11:06 julio4

Hello @julio4 @misicnenad can you review?

Jun 26 '24 13:06 raizo07

Hello @julio4 @misicnenad can you review?

Have you been able to take a look at this? @misicnenad

Jun 28 '24 09:06 raizo07

Hello @julio4 I've implemented all the necessary changes. Kindly review.

Jul 22 '24 09:07 raizo07

@raizo07 I might have to close this PR, I understand that you put lots of effort into it and hopefully learned a lot in the process. However it is still below the quality we are expected. I hope you can understand and feel free to contribute again when you have a more solid understanding of Cairo and Starknet.

Aug 05 '24 04:08 julio4

Hello @julio4 I'm currently resolving all the issues @misicnenad mentioned.

Aug 05 '24 04:08 raizo07

Hello @julio4 I'm currently resolving all the issues @misicnenad mentioned.

I'll keep it open for a while but please take into account comments by @misicnenad, which I agree with.

Aug 08 '24 04:08 julio4