ERC-721: Check ownership in transfer_token_from (#2093)

* ERC-721: Check ownership in `transfer_from`

* Update changelog

* Add test and fix for `transfer()` function

CHANGELOG.md

@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 - Fix the `StorageVec` type by excluding the `len_cached` field from its type info - [#2052](https://github.com/paritytech/ink/pull/2052)
+- ERC-721: `transfer_token_from` now ensures the token owner is correct - [#2093](https://github.com/paritytech/ink/pull/2093)
 
 ## Version 5.0.0-rc
 

integration-tests/erc721/lib.rs

@@ -250,6 +250,9 @@ mod erc721 {
             if !self.approved_or_owner(Some(caller), id) {
                 return Err(Error::NotApproved)
             };
+            if self.token_owner.get(id) != Some(*from) {
+                return Err(Error::NotOwner)
+            };
             self.clear_approval(id);
             self.remove_token_from(from, id)?;
             self.add_token_to(to, id)?;
@@ -630,6 +633,45 @@ mod erc721 {
             assert_eq!(erc721.burn(1), Err(Error::NotOwner));
         }
 
+        #[ink::test]
+        fn transfer_from_fails_not_owner() {
+            let accounts =
+                ink::env::test::default_accounts::<ink::env::DefaultEnvironment>();
+            // Create a new contract instance.
+            let mut erc721 = Erc721::new();
+            // Create token Id 1 for Alice
+            assert_eq!(erc721.mint(1), Ok(()));
+            // Bob can transfer alice's tokens
+            assert_eq!(erc721.set_approval_for_all(accounts.bob, true), Ok(()));
+            // Set caller to Frank
+            set_caller(accounts.frank);
+            // Create token Id 2 for Frank
+            assert_eq!(erc721.mint(2), Ok(()));
+            // Set caller to Bob
+            set_caller(accounts.bob);
+            // Bob makes invalid call to transfer_from (Alice is token owner, not Frank)
+            assert_eq!(
+                erc721.transfer_from(accounts.frank, accounts.bob, 1),
+                Err(Error::NotOwner)
+            );
+        }
+
+        #[ink::test]
+        fn transfer_fails_not_owner() {
+            let accounts =
+                ink::env::test::default_accounts::<ink::env::DefaultEnvironment>();
+            // Create a new contract instance.
+            let mut erc721 = Erc721::new();
+            // Create token Id 1 for Alice
+            assert_eq!(erc721.mint(1), Ok(()));
+            // Bob can transfer alice's tokens
+            assert_eq!(erc721.set_approval_for_all(accounts.bob, true), Ok(()));
+            // Set caller to bob
+            set_caller(accounts.bob);
+            // Bob makes invalid call to transfer (he is not token owner, Alice is)
+            assert_eq!(erc721.transfer(accounts.bob, 1), Err(Error::NotOwner));
+        }
+
         fn set_caller(sender: AccountId) {
             ink::env::test::set_caller::<ink::env::DefaultEnvironment>(sender);
         }