diff --git a/CHANGELOG.md b/CHANGELOG.md index a333a79573..0a44485491 100644 --- a/CHANGELOG.md +++ b/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) +- Fix panic in `approve_for` in the ERC-721 example - [#2092](https://github.com/paritytech/ink/pull/2092) - 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 diff --git a/integration-tests/erc721/lib.rs b/integration-tests/erc721/lib.rs index 7bd80d0699..637836f934 100644 --- a/integration-tests/erc721/lib.rs +++ b/integration-tests/erc721/lib.rs @@ -217,7 +217,7 @@ mod erc721 { let owner = token_owner.get(id).ok_or(Error::TokenNotFound)?; if owner != caller { - return Err(Error::NotOwner) + return Err(Error::NotOwner); }; let count = owned_tokens_count @@ -244,14 +244,12 @@ mod erc721 { id: TokenId, ) -> Result<(), Error> { let caller = self.env().caller(); - if !self.exists(id) { - return Err(Error::TokenNotFound) + let owner = self.owner_of(id).ok_or(Error::TokenNotFound)?; + if !self.approved_or_owner(caller, id, owner) { + return Err(Error::NotApproved); }; - if !self.approved_or_owner(Some(caller), id) { - return Err(Error::NotApproved) - }; - if self.token_owner.get(id) != Some(*from) { - return Err(Error::NotOwner) + if owner != *from { + return Err(Error::NotOwner); }; self.clear_approval(id); self.remove_token_from(from, id)?; @@ -277,7 +275,7 @@ mod erc721 { } = self; if !token_owner.contains(id) { - return Err(Error::TokenNotFound) + return Err(Error::TokenNotFound); } let count = owned_tokens_count @@ -299,11 +297,11 @@ mod erc721 { } = self; if token_owner.contains(id) { - return Err(Error::TokenExists) + return Err(Error::TokenExists); } if *to == AccountId::from([0x0; 32]) { - return Err(Error::NotAllowed) + return Err(Error::NotAllowed); }; let count = owned_tokens_count @@ -325,7 +323,7 @@ mod erc721 { ) -> Result<(), Error> { let caller = self.env().caller(); if to == caller { - return Err(Error::NotAllowed) + return Err(Error::NotAllowed); } self.env().emit_event(ApprovalForAll { owner: caller, @@ -346,19 +344,17 @@ mod erc721 { /// the message's sender. fn approve_for(&mut self, to: &AccountId, id: TokenId) -> Result<(), Error> { let caller = self.env().caller(); - let owner = self.owner_of(id); - if !(owner == Some(caller) - || self.approved_for_all(owner.expect("Error with AccountId"), caller)) - { - return Err(Error::NotAllowed) + let owner = self.owner_of(id).ok_or(Error::TokenNotFound)?; + if !(owner == caller || self.approved_for_all(owner, caller)) { + return Err(Error::NotAllowed); }; if *to == AccountId::from([0x0; 32]) { - return Err(Error::NotAllowed) + return Err(Error::NotAllowed); }; if self.token_approvals.contains(id) { - return Err(Error::CannotInsert) + return Err(Error::CannotInsert); } else { self.token_approvals.insert(id, to); } @@ -389,20 +385,16 @@ mod erc721 { /// Returns true if the `AccountId` `from` is the owner of token `id` /// or it has been approved on behalf of the token `id` owner. - fn approved_or_owner(&self, from: Option, id: TokenId) -> bool { - let owner = self.owner_of(id); - from != Some(AccountId::from([0x0; 32])) + fn approved_or_owner( + &self, + from: AccountId, + id: TokenId, + owner: AccountId, + ) -> bool { + from != AccountId::from([0x0; 32]) && (from == owner - || from == self.token_approvals.get(id) - || self.approved_for_all( - owner.expect("Error with AccountId"), - from.expect("Error with AccountId"), - )) - } - - /// Returns true if token `id` exists or false if it does not. - fn exists(&self, id: TokenId) -> bool { - self.token_owner.contains(id) + || self.token_approvals.get(id) == Some(from) + || self.approved_for_all(owner, from)) } } @@ -563,6 +555,16 @@ mod erc721 { assert!(!erc721.is_approved_for_all(accounts.alice, accounts.bob)); } + #[ink::test] + fn approve_nonexistent_token_should_fail() { + let accounts = + ink::env::test::default_accounts::(); + // Create a new contract instance. + let mut erc721 = Erc721::new(); + // Approve transfer of nonexistent token id 1 + assert_eq!(erc721.approve(accounts.bob, 1), Err(Error::TokenNotFound)); + } + #[ink::test] fn not_approved_transfer_should_fail() { let accounts =