Merge pull request horuslabsio#25 from Darlington02/main

chore: updated _get_owner to be compatible with all contracts

addresses.txt

@@ -19,8 +19,8 @@ Sample TBA:
 MAINNET:
 
 Registry;
-    - ClassHash: 0x26346ce7aa51dcd287ecc0ba3c11ef5436ac84284baa16e4862a684cd1b2830
-    - Contract: 0x117719138a374fc5a9ad8d189fa5daaf089872eb6a2def6472531ac08399c77
+    - ClassHash: 0x2fee320389750aef9d6fe7d95754a75a68d4448c019cee2a34cb6b698972db9
+    - Contract: 0x1b0ef7a47d9db8652f8a9010ecaf3e6537442bfab3afed13449b571fa1da37a
 
 Account:
-    - ClassHash: 0x3b8608dd5be6315eb4eeae62154604ddaccbaa0ad212f6dd088076409baae06
+    - ClassHash: 0x11bc9fabead984d714cf82ec46ffa23f4558f27ae73561542fed9fa8fb510ae

src/account/account.cairo

@@ -175,8 +175,16 @@ mod Account {
         /// @notice internal function for getting NFT owner
         /// @param token_contract contract address of NFT
         // @param token_id token ID of NFT
+        // NB: This function aims for compatibility with all contracts (snake or camel case) but do not work as expected on mainnet as low level calls do not return err at the moment. Should work for contracts which implements CamelCase but not snake_case until starknet v0.15.
         fn _get_owner(self: @ContractState, token_contract: ContractAddress, token_id: u256) -> ContractAddress {
-             IERC721Dispatcher { contract_address: token_contract }.owner_of(token_id)
+            let mut calldata: Array<felt252> = ArrayTrait::new();
+            Serde::serialize(@token_id, ref calldata);
+            let mut res = call_contract_syscall(token_contract, selector!("ownerOf"), calldata.span());
+            if(res.is_err()) {
+                res = call_contract_syscall(token_contract, selector!("owner_of"), calldata.span());
+            }
+            let mut address = res.unwrap();
+            Serde::<ContractAddress>::deserialize(ref address).unwrap()
         }
 
         /// @notice internal transaction for returning the contract address and token ID of the NFT

src/interfaces/IERC721.cairo

@@ -4,6 +4,7 @@ use starknet::ContractAddress;
 trait IERC721<TContractState> {
     fn balance_of(self: @TContractState, account: ContractAddress) -> u256;
     fn owner_of(self: @TContractState, token_id: u256) -> ContractAddress;
+    fn ownerOf(self: @TContractState, token_id: u256) -> ContractAddress;
     fn transfer_from(ref self: TContractState, from: ContractAddress, to: ContractAddress, token_id: u256);
     fn safe_transfer_from(
         ref self: TContractState, from: ContractAddress, to: ContractAddress, token_id: u256, data: Span<felt252>

src/registry/registry.cairo

@@ -3,7 +3,8 @@
 ////////////////////////////////
 #[starknet::contract]
 mod Registry {
-    use core::hash::HashStateTrait;
+    use core::result::ResultTrait;
+use core::hash::HashStateTrait;
     use starknet::{ContractAddress, get_caller_address, syscalls::call_contract_syscall, class_hash::ClassHash, class_hash::Felt252TryIntoClassHash, syscalls::deploy_syscall, SyscallResultTrait};
     use zeroable::Zeroable;
     use traits::{Into, TryInto};
@@ -49,8 +50,8 @@ mod Registry {
             token_contract: ContractAddress,
             token_id: u256,
             salt: felt252
-        ) -> ContractAddress {
-            let owner = IERC721Dispatcher { contract_address: token_contract }.owner_of(token_id);
+        ) -> ContractAddress {    
+            let owner = self._get_owner(token_contract, token_id);  
             assert(owner == get_caller_address(), 'CALLER_IS_NOT_OWNER');
 
             let mut constructor_calldata: Array<felt252> = array![token_contract.into(), token_id.low.into(), token_id.high.into()];
@@ -106,4 +107,22 @@ mod Registry {
             self.registry_deployed_accounts.read((token_contract, token_id))
         }
     }
+
+    #[generate_trait]
+    impl internalImpl of InternalTrait {
+        /// @notice internal function for getting NFT owner
+        /// @param token_contract contract address of NFT
+        // @param token_id token ID of NFT
+        // NB: This function aims for compatibility with all contracts (snake or camel case) but do not work as expected on mainnet as low level calls do not return err at the moment. Should work for contracts which implements CamelCase but not snake_case until starknet v0.15.
+        fn _get_owner(self: @ContractState, token_contract: ContractAddress, token_id: u256) -> ContractAddress {
+            let mut calldata: Array<felt252> = ArrayTrait::new();
+            Serde::serialize(@token_id, ref calldata);
+            let mut res = call_contract_syscall(token_contract, selector!("ownerOf"), calldata.span());
+            if(res.is_err()) {
+                res = call_contract_syscall(token_contract, selector!("owner_of"), calldata.span());
+            }
+            let mut address = res.unwrap();
+            Serde::<ContractAddress>::deserialize(ref address).unwrap()
+        }
+    }
 }

src/test_helper/erc721_helper.cairo

@@ -3,6 +3,7 @@ use starknet::ContractAddress;
 #[starknet::interface]
 trait IERC721<TContractState> {
     fn balance_of(self: @TContractState, account: ContractAddress) -> u256;
+    fn ownerOf(self: @TContractState, token_id: u256) -> ContractAddress;
     fn owner_of(self: @TContractState, token_id: u256) -> ContractAddress;
     fn transfer_from(ref self: TContractState, from: ContractAddress, to: ContractAddress, token_id: u256);
     fn approve(ref self: TContractState, to: ContractAddress, token_id: u256);
@@ -83,6 +84,12 @@ mod ERC721 {
             self.symbol.read()
         }
 
+        fn ownerOf(self: @ContractState, token_id: u256) -> ContractAddress {
+            let owner = self.owners.read(token_id);
+            assert(owner.is_non_zero(), 'ERC721: invalid token ID');
+            owner
+        }
+
         fn owner_of(self: @ContractState, token_id: u256) -> ContractAddress {
             let owner = self.owners.read(token_id);
             assert(owner.is_non_zero(), 'ERC721: invalid token ID');
@@ -108,12 +115,12 @@ mod ERC721 {
         }
 
         fn approve(ref self: ContractState, to: ContractAddress, token_id: u256) {
-            let owner = self.owner_of(token_id);
+            let owner = self.ownerOf(token_id);
             assert(to != owner, 'Approval to current owner');
             assert(get_caller_address() == owner || self.is_approved_for_all(owner, get_caller_address()), 'Not token owner');
             self.token_approvals.write(token_id, to);
             self.emit(
-                Approval{ owner: self.owner_of(token_id), to: to, token_id: token_id }
+                Approval{ owner: self.ownerOf(token_id), to: to, token_id: token_id }
             );
         }
 
@@ -147,7 +154,7 @@ mod ERC721 {
     #[generate_trait]
     impl ERC721HelperImpl of ERC721HelperTrait {
         fn _exists(self: @ContractState, token_id: u256) -> bool {
-            self.owner_of(token_id).is_non_zero()
+            self.ownerOf(token_id).is_non_zero()
         }
 
         fn _is_approved_or_owner(self: @ContractState, spender: ContractAddress, token_id: u256) -> bool {
@@ -163,7 +170,7 @@ mod ERC721 {
         }
 
         fn _transfer(ref self: ContractState, from: ContractAddress, to: ContractAddress, token_id: u256) {
-            assert(from == self.owner_of(token_id), 'ERC721: Caller is not owner');
+            assert(from == self.ownerOf(token_id), 'ERC721: Caller is not owner');
             assert(to.is_non_zero(), 'ERC721: transfer to 0 address');
 
             self.token_approvals.write(token_id, Zeroable::zero());

tests/test_account.cairo

@@ -84,7 +84,7 @@ fn test_is_valid_signature() {
     let hash = data.transaction_hash;
 
     let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address };  
-    let token_owner = token_dispatcher.owner_of(u256_from_felt252(1));
+    let token_owner = token_dispatcher.ownerOf(u256_from_felt252(1));
 
     start_prank(CheatTarget::One(contract_address), token_owner);
     let mut good_signature = array![data.r, data.s];
@@ -121,7 +121,7 @@ fn test_execute() {
 
     // get token owner
     let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address };  
-    let token_owner = token_dispatcher.owner_of(u256_from_felt252(1));
+    let token_owner = token_dispatcher.ownerOf(u256_from_felt252(1));
 
     // start prank
     start_prank(CheatTarget::One(contract_address), token_owner);
@@ -163,7 +163,7 @@ fn test_execute_multicall() {
 
     // get token owner
     let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address };  
-    let token_owner = token_dispatcher.owner_of(u256_from_felt252(1));
+    let token_owner = token_dispatcher.ownerOf(u256_from_felt252(1));
 
     // start prank
     start_prank(CheatTarget::One(contract_address), token_owner);
@@ -194,7 +194,7 @@ fn test_owner() {
     let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address };   
 
     let owner = acct_dispatcher.owner(erc721_contract_address, u256_from_felt252(1));
-    let token_owner = token_dispatcher.owner_of(u256_from_felt252(1));
+    let token_owner = token_dispatcher.ownerOf(u256_from_felt252(1));
     assert(owner == token_owner, 'invalid owner');
 }
 
@@ -207,7 +207,7 @@ fn test_upgrade() {
 
     // get token owner
     let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address };  
-    let token_owner = token_dispatcher.owner_of(u256_from_felt252(1));
+    let token_owner = token_dispatcher.ownerOf(u256_from_felt252(1));
 
     // call the upgrade function
     start_prank(CheatTarget::One(contract_address), token_owner);
@@ -269,7 +269,7 @@ fn test_should_not_execute_when_locked() {
 
     // get token owner
     let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address };  
-    let token_owner = token_dispatcher.owner_of(u256_from_felt252(1));
+    let token_owner = token_dispatcher.ownerOf(u256_from_felt252(1));
 
     // start prank
     start_prank(CheatTarget::One(contract_address), token_owner);
@@ -308,7 +308,7 @@ fn test_should_not_upgrade_when_locked() {
 
     // get token owner
     let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address };  
-    let token_owner = token_dispatcher.owner_of(u256_from_felt252(1));
+    let token_owner = token_dispatcher.ownerOf(u256_from_felt252(1));
 
     // call the upgrade function
     start_prank(CheatTarget::One(contract_address), token_owner);

tests/test_registry.cairo

@@ -45,7 +45,7 @@ fn test_create_account() {
 
     // prank contract as token owner
     let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address };  
-    let token_owner = token_dispatcher.owner_of(u256_from_felt252(1));
+    let token_owner = token_dispatcher.ownerOf(u256_from_felt252(1));
     start_prank(CheatTarget::One(registry_contract_address), token_owner);
 
     // create account
@@ -69,7 +69,7 @@ fn test_getting_total_deployed_accounts() {
 
     // prank contract as token owner
     let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address };  
-    let token_owner = token_dispatcher.owner_of(u256_from_felt252(1));
+    let token_owner = token_dispatcher.ownerOf(u256_from_felt252(1));
     start_prank(CheatTarget::One(registry_contract_address), token_owner);
 
     let acct_class_hash = declare('Account').class_hash;
@@ -90,7 +90,7 @@ fn test_get_account() {
 
     // prank contract as token owner
     let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address };  
-    let token_owner = token_dispatcher.owner_of(u256_from_felt252(1));
+    let token_owner = token_dispatcher.ownerOf(u256_from_felt252(1));
     start_prank(CheatTarget::One(registry_contract_address), token_owner);
 
     // deploy account