Add ERC721 hooks (OpenZeppelin#978)

* feat: working in logic * feat: add empty impl * feat: finish docs * feat: update CHANGELOG * Update src/token/erc721/erc721.cairo Co-authored-by: Andrew Fleming <[email protected]> * Update src/token/erc721/erc721.cairo Co-authored-by: Andrew Fleming <[email protected]> * Update docs/modules/ROOT/pages/api/erc721.adoc Co-authored-by: Andrew Fleming <[email protected]> * feat: apply review updates * docs: update comment * feat: add more tests * feat: apply review updates * feat: apply review updates --------- Co-authored-by: Andrew Fleming <[email protected]>
andrew-fleming · May 14, 2024 · e190e55 · e190e55
1 parent a0899b0
commit e190e55
Show file tree

Hide file tree

Showing 9 changed files with 722 additions and 224 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,10 +10,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- before_update and after_update hooks to ERC721Component (#978)
 - before_update and after_update hooks to ERC1155Component (#982)
 
 ### Changed (Breaking)
 
+- ERC721Component internal implementation to support transfer, mint, and burn flows going through an `_update` function (#978)
+- ERC721Component implementations now require an ERC721HooksTrait implementation in scope.
 - ERC1155Component implementations now require an ERC1155HooksTrait implementation in scope.
 
 ## 0.12.0 (2024-04-21)

diff --git a/docs/modules/ROOT/pages/api/erc721.adoc b/docs/modules/ROOT/pages/api/erc721.adoc
diff --git a/docs/modules/ROOT/pages/erc721.adoc b/docs/modules/ROOT/pages/erc721.adoc
@@ -95,7 +95,7 @@ Here's an example of a basic contract:
 #[starknet::contract]
 mod MyNFT {
     use openzeppelin::introspection::src5::SRC5Component;
-    use openzeppelin::token::erc721::ERC721Component;
+    use openzeppelin::token::erc721::{ERC721Component, ERC721HooksEmptyImpl};
     use starknet::ContractAddress;
 
     component!(path: ERC721Component, storage: erc721, event: ERC721Event);

diff --git a/src/presets/erc721.cairo b/src/presets/erc721.cairo
@@ -12,7 +12,7 @@
 mod ERC721Upgradeable {
     use openzeppelin::access::ownable::OwnableComponent;
     use openzeppelin::introspection::src5::SRC5Component;
-    use openzeppelin::token::erc721::ERC721Component;
+    use openzeppelin::token::erc721::{ERC721Component, ERC721HooksEmptyImpl};
     use openzeppelin::upgrades::UpgradeableComponent;
     use openzeppelin::upgrades::interface::IUpgradeable;
     use starknet::{ContractAddress, ClassHash};
@@ -100,7 +100,6 @@ mod ERC721Upgradeable {
                     break;
                 }
                 let id = *token_ids.pop_front().unwrap();
-
                 self.erc721._mint(recipient, id);
             }
         }

diff --git a/src/tests/mocks/erc721_mocks.cairo b/src/tests/mocks/erc721_mocks.cairo
@@ -1,7 +1,7 @@
 #[starknet::contract]
 mod DualCaseERC721Mock {
     use openzeppelin::introspection::src5::SRC5Component;
-    use openzeppelin::token::erc721::ERC721Component;
+    use openzeppelin::token::erc721::{ERC721Component, ERC721HooksEmptyImpl};
     use starknet::ContractAddress;
 
     component!(path: ERC721Component, storage: erc721, event: ERC721Event);
@@ -57,7 +57,7 @@ mod DualCaseERC721Mock {
 #[starknet::contract]
 mod SnakeERC721Mock {
     use openzeppelin::introspection::src5::SRC5Component;
-    use openzeppelin::token::erc721::ERC721Component;
+    use openzeppelin::token::erc721::{ERC721Component, ERC721HooksEmptyImpl};
     use starknet::ContractAddress;
 
     component!(path: ERC721Component, storage: erc721, event: ERC721Event);
@@ -109,7 +109,7 @@ mod SnakeERC721Mock {
 mod CamelERC721Mock {
     use openzeppelin::introspection::src5::SRC5Component;
     use openzeppelin::token::erc721::ERC721Component::{ERC721Impl, ERC721MetadataImpl};
-    use openzeppelin::token::erc721::ERC721Component;
+    use openzeppelin::token::erc721::{ERC721Component, ERC721HooksEmptyImpl};
     use starknet::ContractAddress;
 
     component!(path: ERC721Component, storage: erc721, event: ERC721Event);

diff --git a/src/tests/presets/test_erc721.cairo b/src/tests/presets/test_erc721.cairo
@@ -269,14 +269,6 @@ fn test_approve_from_unauthorized() {
     dispatcher.approve(SPENDER(), TOKEN_1);
 }
 
-#[test]
-#[should_panic(expected: ('ERC721: approval to owner', 'ENTRYPOINT_FAILED'))]
-fn test_approve_to_owner() {
-    let dispatcher = setup_dispatcher();
-
-    dispatcher.approve(OWNER(), TOKEN_1);
-}
-
 #[test]
 #[should_panic(expected: ('ERC721: invalid token ID', 'ENTRYPOINT_FAILED'))]
 fn test_approve_nonexistent() {
@@ -308,20 +300,6 @@ fn test_set_approval_for_all() {
     assert!(is_not_approved_for_all);
 }
 
-#[test]
-#[should_panic(expected: ('ERC721: self approval', 'ENTRYPOINT_FAILED'))]
-fn test_set_approval_for_all_owner_equal_operator_true() {
-    let dispatcher = setup_dispatcher();
-    dispatcher.set_approval_for_all(OWNER(), true);
-}
-
-#[test]
-#[should_panic(expected: ('ERC721: self approval', 'ENTRYPOINT_FAILED'))]
-fn test_set_approval_for_all_owner_equal_operator_false() {
-    let dispatcher = setup_dispatcher();
-    dispatcher.set_approval_for_all(OWNER(), false);
-}
-
 //
 // transfer_from & transferFrom
 //