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

Feature: ERC721 NFT Contract #250

Merged
julio4 merged 53 commits into from
Mar 7, 2025
Merged

Feature: ERC721 NFT Contract #250

julio4 merged 53 commits into from
Mar 7, 2025

Conversation

@0xNeshi
Copy link
Contributor

@0xNeshi 0xNeshi commented Oct 17, 2024 •
edited
Loading

Issue(s): Close #197

Description

Implement a simple ERC721 contract inspired by OZ's implementation.

Note: should replace non-maintained PR #214

Checklist

0xNeshi added 24 commits October 16, 2024 17:58
@0xNeshi
Add ERC721 initial impl
9703d23
@0xNeshi
Rename token.cairo->erc721.cairo
41bd832
@0xNeshi
Simplify contract
cdc5a9a
@0xNeshi
Implement mint and burn
286e8c4
@0xNeshi
refactor erc721
0b1bb65
@0xNeshi
rename back contract->erc721
eda9fc9
@0xNeshi
Set up tests
446f735
@0xNeshi
Add all getter tests
7a3afc7
@0xNeshi
Return contract_address from deploy
fbb07af
@0xNeshi
Add approve tests
15fa3ae
@0xNeshi
Add transfer_from tests
dceff79
@0xNeshi
Add safe_transfer_from tests
f5ed044
@0xNeshi
Add mint tests
9e7ab1d
@0xNeshi
Add burn tests
4cb576f
@0xNeshi
Add internal tests
282fe6b
@0xNeshi
Move interfaces into interfaces.cairo + fix build errors
e5e84e9
@0xNeshi
fix tests
6bf7d66
@0xNeshi
Fix approvals update in transfer_from
69bc00a
@0xNeshi
Remove redundant build-external-contracts section from Scarb.toml
09b01b7
@0xNeshi
Move snforge_std dep to dev deps
9e79fb3
@0xNeshi
Set edition to workspace version
20b34b1
@0xNeshi
Revert edition to specific version + remove [lib]
aa3c4a2
@0xNeshi
Merge remote-tracking branch 'origin/main' into erc721
fef0cd3
@0xNeshi
Update edition to point to workspace
0650e1e
@julio4 julio4 mentioned this pull request Nov 4, 2024
Closed
@julio4
Copy link
Member

julio4 commented Nov 20, 2024

@0xNeshi The repository has been upgraded to a new site generator and the Markdown format to include code listings is slightly different. The CONTRIBUTING guidelines has been updated to reflect these changes.
Could you rebase this PR onto dev. If there's any issues or if you want me to do these changes let me know.

@julio4 julio4 changed the base branch from main to dev November 20, 2024 03:33
julio4
julio4 reviewed Nov 20, 2024
View reviewed changes
Copy link
Member

@julio4 julio4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, a few small changes to follow the IERC721 as much as possible!

src/applications/erc721.md Outdated
{{#include ../../listings/applications/721/src/erc721.cairo}}
```

There are other implementations, such as the [Open Zeppelin](https://docs.openzeppelin.com/contracts-cairo/0.7.0/erc721) one.
Copy link
Member

@julio4 julio4 Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update to https://docs.openzeppelin.com/contracts-cairo/0.19.0/erc721

Copy link
Contributor Author

@0xNeshi 0xNeshi Dec 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to 0.20.0 as the most recent one, see https://docs.openzeppelin.com/contracts-cairo/0.20.0/erc721

Updated the link for erc20 too, see 4e9607c

Copy link
Contributor Author

@0xNeshi 0xNeshi Mar 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@julio4 removed version (link still works), see e3d04fa

src/applications/erc721.md Outdated
@@ -0,0 +1,19 @@
# ERC721 Token

Contracts that follow the [ERC721 Standard](https://eips.ethereum.org/EIPS/eip-721) are called ERC721 tokens. They are used to represent non-fungible assets.
Copy link
Member

@julio4 julio4 Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a small comment to recommend reader to actually click and read the EIP to learn about all the erc721 interface specifications

Copy link
Contributor Author

@0xNeshi 0xNeshi Dec 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

b92ae56

@julio4
Prepare release 2.8.2 (NethermindEth#257)
badf4db 
* dep: fix patch to latest shikijs for cairo hl

* fix(app): correct sidebar placement

* fix(app): responsive content centering

* fix(app): responsive content centering

* doc: branch guidelines

* fix(app): top section nav links
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
listings/applications/erc721/src/tests.cairo Outdated
Comment on lines 456 to 458
Event::Transfer(Transfer { from: owner, to: account, token_id: TOKEN_ID })
)
]
Copy link
Contributor Author

@0xNeshi 0xNeshi Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Event::Transfer(Transfer { from: owner, to: account, token_id: TOKEN_ID })
)
]
Event::Transfer(Transfer { from: owner, to: account, token_id: TOKEN_ID }),
),
],

0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
listings/applications/erc721/src/tests.cairo Outdated
Comment on lines 480 to 482
Event::Transfer(Transfer { from: owner, to: receiver, token_id: TOKEN_ID })
)
]
Copy link
Contributor Author

@0xNeshi 0xNeshi Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Event::Transfer(Transfer { from: owner, to: receiver, token_id: TOKEN_ID })
)
]
Event::Transfer(Transfer { from: owner, to: receiver, token_id: TOKEN_ID }),
),
],

0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
listings/applications/erc721/src/tests.cairo Outdated
}

#[test]
#[should_panic(expected: ('ENTRYPOINT_NOT_FOUND', 'ENTRYPOINT_FAILED',))]
Copy link
Contributor Author

@0xNeshi 0xNeshi Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#[should_panic(expected: ('ENTRYPOINT_NOT_FOUND', 'ENTRYPOINT_FAILED',))]
#[should_panic(expected: ('ENTRYPOINT_NOT_FOUND', 'ENTRYPOINT_FAILED'))]

0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
listings/applications/erc721/src/tests.cairo Outdated
Comment on lines 543 to 545
Event::Transfer(Transfer { from: owner, to: owner, token_id: TOKEN_ID })
)
]
Copy link
Contributor Author

@0xNeshi 0xNeshi Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Event::Transfer(Transfer { from: owner, to: owner, token_id: TOKEN_ID })
)
]
Event::Transfer(Transfer { from: owner, to: owner, token_id: TOKEN_ID }),
),
],

0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
listings/applications/erc721/src/tests.cairo Outdated
Comment on lines 572 to 574
Event::Transfer(Transfer { from: owner, to: receiver, token_id: TOKEN_ID })
)
]
Copy link
Contributor Author

@0xNeshi 0xNeshi Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Event::Transfer(Transfer { from: owner, to: receiver, token_id: TOKEN_ID })
)
]
Event::Transfer(Transfer { from: owner, to: receiver, token_id: TOKEN_ID }),
),
],

0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
listings/applications/erc721/src/tests.cairo Outdated
Comment on lines 600 to 602
Event::Transfer(Transfer { from: owner, to: receiver, token_id: TOKEN_ID })
)
]
Copy link
Contributor Author

@0xNeshi 0xNeshi Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Event::Transfer(Transfer { from: owner, to: receiver, token_id: TOKEN_ID })
)
]
Event::Transfer(Transfer { from: owner, to: receiver, token_id: TOKEN_ID }),
),
],

0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
listings/applications/erc721/src/tests.cairo Outdated
Comment on lines 638 to 640
Event::Transfer(Transfer { from: ZERO(), to: recipient, token_id: TOKEN_ID })
)
]
Copy link
Contributor Author

@0xNeshi 0xNeshi Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Event::Transfer(Transfer { from: ZERO(), to: recipient, token_id: TOKEN_ID })
)
]
Event::Transfer(Transfer { from: ZERO(), to: recipient, token_id: TOKEN_ID }),
),
],

0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
0xNeshi
0xNeshi commented Mar 5, 2025
View reviewed changes
@julio4 julio4 merged commit f50bfe4 into NethermindEth:dev Mar 7, 2025
2 checks passed
@0xNeshi 0xNeshi deleted the erc721 branch March 7, 2025 19:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@julio4 julio4 julio4 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Feature: ERC721 NFT Contract

2 participants

@0xNeshi @julio4