Use of namespaces for ERC721 & ERC721Enumerable (#296)

* refactor erc20 around the _spendAllowance function * update ERC20 tests around spendAllowance * use false instead of 0 * change naming scheme * using namespace * fix mock * constructor name * fix imports * fix case * Refactor ERC721 * fix testing with nile 0.6.1 * fix tests * address PR comments * update docs * Apply suggestions from code review Co-authored-by: Martín Triay <martriay@gmail.com> * fix merge * address PR comments * move private function outside the namespace * Update src/openzeppelin/token/erc721_enumerable/library.cairo Co-authored-by: Martín Triay <martriay@gmail.com> Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com>
OpenZeppelin · Jun 16, 2022 · b27101e · b27101e
1 parent cdaabf6
commit b27101e
Show file tree

Hide file tree

Showing 7 changed files with 603 additions and 676 deletions.
diff --git a/docs/ERC721.md b/docs/ERC721.md
@@ -241,24 +241,24 @@ Ready-to-use presets are a great option for testing and prototyping. See [Preset
 
 ## Extensibility
 
-Following the [contracts extensibility pattern](Extensibility.md), this implementation is set up to include all ERC721 storage and function logic within the `ERC721_base.cairo` library. Library methods with the prefix `ERC721_` must be imported in the smart contract and inserted into an `external` method with the requisite name. This is already done in the [preset contracts](#presets); however, additional functionality can be added. For instance, you could:
+Following the [contracts extensibility pattern](Extensibility.md), this implementation is set up to include all ERC721 related storage and business logic under a namespace. Developers should be mindful of manually exposing the required methods from the namespace to comply with the standard interface. This is already done in the [preset contracts](#presets); however, additional functionality can be added. For instance, you could:
 
 - Implement a pausing mechanism
 - Add roles such as _owner_ or _minter_
 - Modify the `transferFrom` function to mimic the [`_beforeTokenTransfer` and `_afterTokenTransfer` hooks](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/ERC721.sol#L335)
 
-Just be sure that the exposed `external` methods invoke their imported function logic a la `approve` invokes `ERC721_approve`. As an example, see below.
+Just be sure that the exposed `external` methods invoke their imported function logic a la `approve` invokes `ERC721.approve`. As an example, see below.
 
 ```python
-from openzeppelin.token.erc721.library import ERC721_approve
+from openzeppelin.token.erc721.library import ERC721
 
 @external
 func approve{
         pedersen_ptr: HashBuiltin*,
         syscall_ptr: felt*,
         range_check_ptr
     }(to: felt, tokenId: Uint256):
-    ERC721_approve(to, tokenId)
+    ERC721.approve(to, tokenId)
     return()
 end
 
@@ -280,18 +280,18 @@ The `ERC721_Mintable_Pausable` preset creates a contract with pausable token tra
 
 The `ERC721_Enumerable_Mintable_Burnable` preset adds enumerability of all the token ids in the contract as well as all token ids owned by each account. This allows contracts to publish its full list of NFTs and make them discoverable.
 
-In regard to implementation, contracts should import the following view methods:
+In regard to implementation, contracts should expose the following view methods:
 
-- `ERC721_Enumerable_totalSupply`
-- `ERC721_Enumerable_tokenByIndex`
-- `ERC721_Enumerable_tokenOfOwnerByIndex`
+- `ERC721_Enumerable.total_supply`
+- `ERC721_Enumerable.token_by_index`
+- `ERC721_Enumerable.token_of_owner_by_index`
 
-In order for the tokens to be correctly indexed, the contract should also import the following methods (which supercede some of the `ERC721_base` methods):
+In order for the tokens to be correctly indexed, the contract should also use the following methods (which supercede some of the base `ERC721` methods):
 
-- `ERC721_Enumerable_transferFrom`
-- `ERC721_Enumerable_safeTransferFrom`
-- `ERC721_Enumerable_mint`
-- `ERC721_Enumerable_burn`
+- `ERC721_Enumerable.transfer_from`
+- `ERC721_Enumerable.safe_transfer_from`
+- `ERC721_Enumerable._mint`
+- `ERC721_Enumerable._burn`
 
 #### IERC721_Enumerable
 

diff --git a/src/openzeppelin/token/erc721/ERC721_Mintable_Burnable.cairo b/src/openzeppelin/token/erc721/ERC721_Mintable_Burnable.cairo
@@ -6,26 +6,7 @@
 from starkware.cairo.common.cairo_builtins import HashBuiltin, SignatureBuiltin
 from starkware.cairo.common.uint256 import Uint256
 
-from openzeppelin.token.erc721.library import (
-    ERC721_name,
-    ERC721_symbol,
-    ERC721_balanceOf,
-    ERC721_ownerOf,
-    ERC721_getApproved,
-    ERC721_isApprovedForAll,
-    ERC721_tokenURI,
-
-    ERC721_initializer,
-    ERC721_approve,
-    ERC721_setApprovalForAll,
-    ERC721_transferFrom,
-    ERC721_safeTransferFrom,
-    ERC721_mint,
-    ERC721_burn,
-    ERC721_only_token_owner,
-    ERC721_setTokenURI
-)
-
+from openzeppelin.token.erc721.library import ERC721
 from openzeppelin.introspection.ERC165 import ERC165
 
 from openzeppelin.access.ownable import Ownable
@@ -44,7 +25,7 @@ func constructor{
         symbol: felt,
         owner: felt
     ):
-    ERC721_initializer(name, symbol)
+    ERC721.initializer(name, symbol)
     Ownable.initializer(owner)
     return ()
 end
@@ -69,7 +50,7 @@ func name{
         pedersen_ptr : HashBuiltin*,
         range_check_ptr
     }() -> (name: felt):
-    let (name) = ERC721_name()
+    let (name) = ERC721.name()
     return (name)
 end
 
@@ -79,7 +60,7 @@ func symbol{
         pedersen_ptr : HashBuiltin*,
         range_check_ptr
     }() -> (symbol: felt):
-    let (symbol) = ERC721_symbol()
+    let (symbol) = ERC721.symbol()
     return (symbol)
 end
 
@@ -89,7 +70,7 @@ func balanceOf{
         pedersen_ptr : HashBuiltin*,
         range_check_ptr
     }(owner: felt) -> (balance: Uint256):
-    let (balance: Uint256) = ERC721_balanceOf(owner)
+    let (balance: Uint256) = ERC721.balance_of(owner)
     return (balance)
 end
 
@@ -99,7 +80,7 @@ func ownerOf{
         pedersen_ptr : HashBuiltin*,
         range_check_ptr
     }(tokenId: Uint256) -> (owner: felt):
-    let (owner: felt) = ERC721_ownerOf(tokenId)
+    let (owner: felt) = ERC721.owner_of(tokenId)
     return (owner)
 end
 
@@ -109,7 +90,7 @@ func getApproved{
         pedersen_ptr : HashBuiltin*,
         range_check_ptr
     }(tokenId: Uint256) -> (approved: felt):
-    let (approved: felt) = ERC721_getApproved(tokenId)
+    let (approved: felt) = ERC721.get_approved(tokenId)
     return (approved)
 end
 
@@ -119,7 +100,7 @@ func isApprovedForAll{
         pedersen_ptr : HashBuiltin*,
         range_check_ptr
     }(owner: felt, operator: felt) -> (isApproved: felt):
-    let (isApproved: felt) = ERC721_isApprovedForAll(owner, operator)
+    let (isApproved: felt) = ERC721.is_approved_for_all(owner, operator)
     return (isApproved)
 end
 
@@ -129,7 +110,7 @@ func tokenURI{
         pedersen_ptr: HashBuiltin*,
         range_check_ptr
     }(tokenId: Uint256) -> (tokenURI: felt):
-    let (tokenURI: felt) = ERC721_tokenURI(tokenId)
+    let (tokenURI: felt) = ERC721.token_uri(tokenId)
     return (tokenURI)
 end
 
@@ -153,7 +134,7 @@ func approve{
         syscall_ptr: felt*,
         range_check_ptr
     }(to: felt, tokenId: Uint256):
-    ERC721_approve(to, tokenId)
+    ERC721.approve(to, tokenId)
     return ()
 end
 
@@ -163,7 +144,7 @@ func setApprovalForAll{
         pedersen_ptr: HashBuiltin*,
         range_check_ptr
     }(operator: felt, approved: felt):
-    ERC721_setApprovalForAll(operator, approved)
+    ERC721.set_approval_for_all(operator, approved)
     return ()
 end
 
@@ -177,7 +158,7 @@ func transferFrom{
         to: felt,
         tokenId: Uint256
     ):
-    ERC721_transferFrom(from_, to, tokenId)
+    ERC721.transfer_from(from_, to, tokenId)
     return ()
 end
 
@@ -193,7 +174,7 @@ func safeTransferFrom{
         data_len: felt,
         data: felt*
     ):
-    ERC721_safeTransferFrom(from_, to, tokenId, data_len, data)
+    ERC721.safe_transfer_from(from_, to, tokenId, data_len, data)
     return ()
 end
 
@@ -204,7 +185,7 @@ func mint{
         range_check_ptr
     }(to: felt, tokenId: Uint256):
     Ownable.assert_only_owner()
-    ERC721_mint(to, tokenId)
+    ERC721._mint(to, tokenId)
     return ()
 end
 
@@ -214,8 +195,8 @@ func burn{
         syscall_ptr: felt*,
         range_check_ptr
     }(tokenId: Uint256):
-    ERC721_only_token_owner(tokenId)
-    ERC721_burn(tokenId)
+    ERC721.assert_only_token_owner(tokenId)
+    ERC721._burn(tokenId)
     return ()
 end
 
@@ -226,7 +207,7 @@ func setTokenURI{
         range_check_ptr
     }(tokenId: Uint256, tokenURI: felt):
     Ownable.assert_only_owner()
-    ERC721_setTokenURI(tokenId, tokenURI)
+    ERC721._set_token_uri(tokenId, tokenURI)
     return ()
 end
 

diff --git a/src/openzeppelin/token/erc721/ERC721_Mintable_Pausable.cairo b/src/openzeppelin/token/erc721/ERC721_Mintable_Pausable.cairo
@@ -6,24 +6,7 @@
 from starkware.cairo.common.cairo_builtins import HashBuiltin, SignatureBuiltin
 from starkware.cairo.common.uint256 import Uint256
 
-from openzeppelin.token.erc721.library import (
-    ERC721_name,
-    ERC721_symbol,
-    ERC721_balanceOf,
-    ERC721_ownerOf,
-    ERC721_getApproved,
-    ERC721_isApprovedForAll,
-    ERC721_tokenURI,
-
-    ERC721_initializer,
-    ERC721_approve,
-    ERC721_setApprovalForAll,
-    ERC721_transferFrom,
-    ERC721_safeTransferFrom,
-    ERC721_mint,
-    ERC721_setTokenURI
-)
-
+from openzeppelin.token.erc721.library import ERC721
 from openzeppelin.introspection.ERC165 import ERC165
 
 from openzeppelin.security.pausable import Pausable
@@ -44,7 +27,7 @@ func constructor{
         symbol: felt,
         owner: felt
     ):
-    ERC721_initializer(name, symbol)
+    ERC721.initializer(name, symbol)
     Ownable.initializer(owner)
     return ()
 end
@@ -69,7 +52,7 @@ func name{
         pedersen_ptr : HashBuiltin*,
         range_check_ptr
     }() -> (name: felt):
-    let (name) = ERC721_name()
+    let (name) = ERC721.name()
     return (name)
 end
 
@@ -79,7 +62,7 @@ func symbol{
         pedersen_ptr : HashBuiltin*,
         range_check_ptr
     }() -> (symbol: felt):
-    let (symbol) = ERC721_symbol()
+    let (symbol) = ERC721.symbol()
     return (symbol)
 end
 
@@ -89,7 +72,7 @@ func balanceOf{
         pedersen_ptr : HashBuiltin*,
         range_check_ptr
     }(owner: felt) -> (balance: Uint256):
-    let (balance: Uint256) = ERC721_balanceOf(owner)
+    let (balance: Uint256) = ERC721.balance_of(owner)
     return (balance)
 end
 
@@ -99,7 +82,7 @@ func ownerOf{
         pedersen_ptr : HashBuiltin*,
         range_check_ptr
     }(tokenId: Uint256) -> (owner: felt):
-    let (owner: felt) = ERC721_ownerOf(tokenId)
+    let (owner: felt) = ERC721.owner_of(tokenId)
     return (owner)
 end
 
@@ -109,7 +92,7 @@ func getApproved{
         pedersen_ptr : HashBuiltin*,
         range_check_ptr
     }(tokenId: Uint256) -> (approved: felt):
-    let (approved: felt) = ERC721_getApproved(tokenId)
+    let (approved: felt) = ERC721.get_approved(tokenId)
     return (approved)
 end
 
@@ -119,7 +102,7 @@ func isApprovedForAll{
         pedersen_ptr : HashBuiltin*,
         range_check_ptr
     }(owner: felt, operator: felt) -> (isApproved: felt):
-    let (isApproved: felt) = ERC721_isApprovedForAll(owner, operator)
+    let (isApproved: felt) = ERC721.is_approved_for_all(owner, operator)
     return (isApproved)
 end
 
@@ -129,7 +112,7 @@ func tokenURI{
         pedersen_ptr: HashBuiltin*,
         range_check_ptr
     }(tokenId: Uint256) -> (tokenURI: felt):
-    let (tokenURI: felt) = ERC721_tokenURI(tokenId)
+    let (tokenURI: felt) = ERC721.token_uri(tokenId)
     return (tokenURI)
 end
 
@@ -164,7 +147,7 @@ func approve{
         range_check_ptr
     }(to: felt, tokenId: Uint256):
     Pausable.assert_not_paused()
-    ERC721_approve(to, tokenId)
+    ERC721.approve(to, tokenId)
     return ()
 end
 
@@ -175,7 +158,7 @@ func setApprovalForAll{
         range_check_ptr
     }(operator: felt, approved: felt):
     Pausable.assert_not_paused()
-    ERC721_setApprovalForAll(operator, approved)
+    ERC721.set_approval_for_all(operator, approved)
     return ()
 end
 
@@ -190,7 +173,7 @@ func transferFrom{
         tokenId: Uint256
     ):
     Pausable.assert_not_paused()
-    ERC721_transferFrom(from_, to, tokenId)
+    ERC721.transfer_from(from_, to, tokenId)
     return ()
 end
 
@@ -207,7 +190,7 @@ func safeTransferFrom{
         data: felt*
     ):
     Pausable.assert_not_paused()
-    ERC721_safeTransferFrom(from_, to, tokenId, data_len, data)
+    ERC721.safe_transfer_from(from_, to, tokenId, data_len, data)
     return ()
 end
 
@@ -219,7 +202,7 @@ func mint{
     }(to: felt, tokenId: Uint256):
     Pausable.assert_not_paused()
     Ownable.assert_only_owner()
-    ERC721_mint(to, tokenId)
+    ERC721._mint(to, tokenId)
     return ()
 end
 
@@ -230,7 +213,7 @@ func setTokenURI{
         range_check_ptr
     }(tokenId: Uint256, tokenURI: felt):
     Ownable.assert_only_owner()
-    ERC721_setTokenURI(tokenId, tokenURI)
+    ERC721._set_token_uri(tokenId, tokenURI)
     return ()
 end