fix(L-02): Inconsistent Implementations in ERC-721 Extension #331

.github/workflows/check-links.yml

@@ -15,16 +15,14 @@ jobs:
  runs-on: ubuntu-latest
  steps:
  - uses: actions/checkout@v4
- 
+
  - name: Link Checker
  uses: lycheeverse/lychee-action@v1
  with:
  args: --no-progress './**/*.md'
  fail: true
- 
+
  - name: Run linkspector
  uses: umbrelladocs/action-linkspector@v1
  with:
  fail_on_error: true
-
-

contracts/src/token/erc721/extensions/metadata.rs

@@ -1,12 +1,17 @@
 //! Optional Metadata of the ERC-721 standard.
 
-use alloc::string::String;
+use alloc::string::{String, ToString};
 
-use alloy_primitives::FixedBytes;
-use openzeppelin_stylus_proc::interface_id;
+use alloy_primitives::{FixedBytes, U256};
 use stylus_proc::{public, sol_storage};
 
-use crate::utils::{introspection::erc165::IErc165, Metadata};
+use crate::{
+ token::{
+ erc20::extensions::IErc20Metadata,
+ erc721::{Error, IErc721},
+ },
+ utils::{introspection::erc165::IErc165, Metadata},
+};
 
 sol_storage! {
  /// Metadata of an [`crate::token::erc721::Erc721`] token.
@@ -19,7 +24,6 @@
 }
 
 /// Interface for the optional metadata functions from the ERC-721 standard.
-#[interface_id]
 pub trait IErc721Metadata {
  /// Returns the token collection name.
  ///
@@ -34,14 +38,6 @@
  ///
  /// * `&self` - Read access to the contract's state.
  fn symbol(&self) -> String;
-
- /// Returns the base of Uniform Resource Identifier (URI) for tokens'
- /// collection.
- ///
- /// # Arguments
- ///
- /// * `&self` - Read access to the contract's state.
- fn base_uri(&self) -> String;
 }
 
 // FIXME: Apply multi-level inheritance to export Metadata's functions.
@@ -56,29 +52,65 @@
  fn symbol(&self) -> String {
  self._metadata.symbol()
  }
-
- fn base_uri(&self) -> String {
- self._base_uri.get_string()
- }
 }
 
 impl IErc165 for Erc721Metadata {
  fn supports_interface(interface_id: FixedBytes<4>) -> bool {
- <Self as IErc721Metadata>::INTERFACE_ID
- == u32::from_be_bytes(*interface_id)
+ // NOTE: interface id is calculated using additional selector
+ // [`Erc721Metadata::token_uri`]
+ 0x_5b5e139f == u32::from_be_bytes(*interface_id)
  }
 }
 
-#[cfg(all(test, feature = "std"))]
-mod tests {
- // use crate::token::erc721::extensions::{Erc721Metadata, IErc721Metadata};
+impl Erc721Metadata {
+ /// Returns the base of Uniform Resource Identifier (URI) for tokens'
+ /// collection.
+ ///
+ /// # Arguments
+ ///
+ /// * `&self` - Read access to the contract's state.
+ pub fn base_uri(&self) -> String {
+ self._base_uri.get_string()
+ }
 
- // TODO: IErc721Metadata should be refactored to have same api as solidity
- // has: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/4764ea50750d8bda9096e833706beba86918b163/contracts/token/ERC721/extensions/IERC721Metadata.sol#L12
- // [motsu::test]
- // fn interface_id() {
- // let actual = <Erc721Metadata as IErc721Metadata>::INTERFACE_ID;
- // let expected = 0x5b5e139f;
- // assert_eq!(actual, expected);
- // }
+ /// Returns the Uniform Resource Identifier (URI) for `token_id` token.
+ ///
+ /// # Arguments
+ ///
+ /// * `&self` - Read access to the contract's state.
+ /// * `token_id` - Id of a token.
+ /// * `erc721` - Read access to a contract providing [`IErc721`] interface.
+ ///
+ /// # Errors
+ ///
+ /// If the token does not exist, then the error
+ /// [`Error::NonexistentToken`] is returned.
+ ///
+ /// NOTE: In order to have [`Erc721Metadata::token_uri`] exposed in ABI,
+ /// you need to do this manually.
+ ///
+ /// # Examples
+ ///
+ /// ```rust,ignore
+ /// #[selector(name = "tokenURI")]
+ /// pub fn token_uri(&self, token_id: U256) -> Result<String, Vec<u8>> {
+ /// Ok(self.metadata.token_uri(token_id, &self.erc721)?)
+ /// }
+ pub fn token_uri(
+ &self,
+ token_id: U256,
+ erc721: &impl IErc721<Error = Error>,
+ ) -> Result<String, Error> {
+ erc721.owner_of(token_id)?;
+
+ let base_uri = self.base_uri();
+
+ let token_uri = if base_uri.is_empty() {
+ String::new()
+ } else {
+ base_uri + &token_id.to_string()
+ };
+
+ Ok(token_uri)
+ }
 }

contracts/src/token/erc721/extensions/uri_storage.rs

@@ -5,9 +5,11 @@ use alloc::string::String;
 
 use alloy_primitives::U256;
 use alloy_sol_types::sol;
-use stylus_proc::{public, sol_storage};
+use stylus_proc::sol_storage;
 use stylus_sdk::evm;
 
+use crate::token::erc721::{extensions::Erc721Metadata, Error, IErc721};
+
 sol! {
  /// This event gets emitted when the metadata of a token is changed.
  ///
@@ -46,53 +48,138 @@ impl Erc721UriStorage {
  self._token_uris.setter(token_id).set_str(token_uri);
  evm::log(MetadataUpdate { token_id });
  }
-}
 
-#[public]
-impl Erc721UriStorage {
  /// Returns the Uniform Resource Identifier (URI) for `token_id` token.
  ///
  /// # Arguments
  ///
  /// * `&self` - Read access to the contract's state.
  /// * `token_id` - Id of a token.
- #[must_use]
- pub fn token_uri(&self, token_id: U256) -> String {
- self._token_uris.getter(token_id).get_string()
+ /// * `erc721` - Read access to a contract providing [`IErc721`] interface.
+ /// * `metadata` - Read access to a [`Erc721Metadata`] contract.
+ ///
+ /// # Errors
+ ///
+ /// If the token does not exist, then the error
+ /// [`Error::NonexistentToken`] is returned.
+ ///
+ /// NOTE: In order to have [`Erc721UriStorage::token_uri`] exposed in ABI,
+ /// you need to do this manually.
+ ///
+ /// # Examples
+ ///
+ /// ```rust,ignore
+ /// #[selector(name = "tokenURI")]
+ /// pub fn token_uri(&self, token_id: U256) -> Result<String, Vec<u8>> {
+ /// Ok(self.uri_storage.token_uri(
+ /// token_id,
+ /// &self.erc721,
+ /// &self.metadata,
+ /// )?)
+ /// }
+ pub fn token_uri(
+ &self,
+ token_id: U256,
+ erc721: &impl IErc721<Error = Error>,
+ metadata: &Erc721Metadata,
+ ) -> Result<String, Error> {
+ let _owner = erc721.owner_of(token_id)?;
+
+ let token_uri = self._token_uris.getter(token_id).get_string();
+ let base = metadata.base_uri();
+
+ // If there is no base URI, return the token URI.
+ if base.is_empty() {
+ return Ok(token_uri);
+ }
+
+ // If both are set, concatenate the `base_uri` and `token_uri`.
+ let uri = if !token_uri.is_empty() {
+ base + &token_uri
+ } else {
+ metadata.token_uri(token_id, erc721)?
+ };
+
+ Ok(uri)
  }
 }
 
 #[cfg(all(test, feature = "std"))]
 mod tests {
  use alloy_primitives::U256;
+ use stylus_proc::sol_storage;
+ use stylus_sdk::msg;
 
  use super::Erc721UriStorage;
+ use crate::token::erc721::{extensions::Erc721Metadata, Erc721};
 
  fn random_token_id() -> U256 {
  let num: u32 = rand::random();
  U256::from(num)
  }
 
+ sol_storage! {
+ struct Erc721MetadataExample {
+ Erc721 erc721;
+ Erc721Metadata metadata;
+ Erc721UriStorage uri_storage;
+ }
+ }
+
  #[motsu::test]
- fn get_token_uri_works(contract: Erc721UriStorage) {
+ fn get_token_uri_works(contract: Erc721MetadataExample) {
+ let alice = msg::sender();
+
  let token_id = random_token_id();
 
- let token_uri = String::from("https://docs.openzeppelin.com/contracts/5.x/api/token/erc721#Erc721URIStorage");
- contract._token_uris.setter(token_id).set_str(token_uri.clone());
+ contract
+ .erc721
+ ._mint(alice, token_id)
+ .expect("should mint a token for Alice");
 
- assert_eq!(token_uri, contract.token_uri(token_id));
+ let token_uri = String::from("https://docs.openzeppelin.com/contracts/5.x/api/token/erc721#Erc721URIStorage");
+ contract
+ .uri_storage
+ ._token_uris
+ .setter(token_id)
+ .set_str(token_uri.clone());
+
+ assert_eq!(
+ token_uri,
+ contract
+ .uri_storage
+ .token_uri(token_id, &contract.erc721, &contract.metadata)
+ .expect("should return token URI")
+ );
  }
 
  #[motsu::test]
- fn set_token_uri_works(contract: Erc721UriStorage) {
+ fn set_token_uri_works(contract: Erc721MetadataExample) {
+ let alice = msg::sender();
+
  let token_id = random_token_id();
 
+ contract
+ .erc721
+ ._mint(alice, token_id)
+ .expect("should mint a token for Alice");
+
  let initial_token_uri = String::from("https://docs.openzeppelin.com/contracts/5.x/api/token/erc721#Erc721URIStorage");
- contract._token_uris.setter(token_id).set_str(initial_token_uri);
+ contract
+ .uri_storage
+ ._token_uris
+ .setter(token_id)
+ .set_str(initial_token_uri);
 
  let token_uri = String::from("Updated Token URI");
- contract._set_token_uri(token_id, token_uri.clone());
-
- assert_eq!(token_uri, contract.token_uri(token_id));
+ contract.uri_storage._set_token_uri(token_id, token_uri.clone());
+
+ assert_eq!(
+ token_uri,
+ contract
+ .uri_storage
+ .token_uri(token_id, &contract.erc721, &contract.metadata)
+ .expect("should return token URI")
+ );
  }
 }

docs/modules/ROOT/pages/crypto.adoc

@@ -22,6 +22,6 @@ pub fn verify(&self, proof: Vec<B256>, root: B256, leaf: B256) -> bool {
 }
 ----
 
-Note that these functions use `keccak256` as the hashing algorithm, but our library also provides generic counterparts: https://docs.rs/openzeppelin-crypto/0.1.0-rc/openzeppelin_crypto/merkle/struct.Verifier.html#method.verify_with_builder[`verify_with_builer`] and https://docs.rs/openzeppelin-crypto/0.1.0-rc/openzeppelin_crypto/merkle/struct.Verifier.html#method.verify_multi_proof_with_builder[`verify_multi_proof_with_builder`].
+Note that these functions use `keccak256` as the hashing algorithm, but our library also provides generic counterparts: https://docs.rs/openzeppelin-crypto/0.1.0-rc/openzeppelin_crypto/merkle/struct.Verifier.html#method.verify_with_builder[`verify_with_builder`] and https://docs.rs/openzeppelin-crypto/0.1.0-rc/openzeppelin_crypto/merkle/struct.Verifier.html#method.verify_multi_proof_with_builder[`verify_multi_proof_with_builder`].
 
 We also provide an adapter https://docs.rs/openzeppelin-crypto/0.1.0-rc/openzeppelin_crypto/hash/index.html[`hash`] module to use your own hashers in conjunction with them that resembles Rust's standard library's API.

examples/erc721-metadata/src/lib.rs

@@ -1,18 +1,15 @@
 #![cfg_attr(not(test), no_main, no_std)]
 extern crate alloc;
 
-use alloc::{
- string::{String, ToString},
- vec::Vec,
-};
+use alloc::{string::String, vec::Vec};
 
 use alloy_primitives::{Address, U256};
 use openzeppelin_stylus::token::erc721::{
  extensions::{
  Erc721Metadata as Metadata, Erc721UriStorage as UriStorage,
- IErc721Burnable, IErc721Metadata,
+ IErc721Burnable,
  },
- Erc721, IErc721,
+ Erc721,
 };
 use stylus_sdk::prelude::{entrypoint, public, sol_storage};
 
@@ -23,13 +20,12 @@ sol_storage! {
  Erc721 erc721;
  #[borrow]
  Metadata metadata;
- #[borrow]
  UriStorage uri_storage;
  }
 }
 
 #[public]
-#[inherit(Erc721, Metadata, UriStorage)]
+#[inherit(Erc721, Metadata)]
 impl Erc721MetadataExample {
  pub fn mint(&mut self, to: Address, token_id: U256) -> Result<(), Vec<u8>> {
  Ok(self.erc721._mint(to, token_id)?)
@@ -39,29 +35,13 @@ impl Erc721MetadataExample {
  Ok(self.erc721.burn(token_id)?)
  }
 
- // Overrides [`Erc721UriStorage::token_uri`].
- // Returns the Uniform Resource Identifier (URI) for tokenId token.
  #[selector(name = "tokenURI")]
  pub fn token_uri(&self, token_id: U256) -> Result<String, Vec<u8>> {
- let _owner = self.erc721.owner_of(token_id)?;
-
- let base = self.metadata.base_uri();
- let token_uri = self.uri_storage.token_uri(token_id);
-
- // If there is no base URI, return the token URI.
- if base.is_empty() {
- return Ok(token_uri);
- }
-
- // If both are set,
- // concatenate the base URI and token URI.
- let uri = if !token_uri.is_empty() {
- base + &token_uri
- } else {
- base + &token_id.to_string()
- };
-
- Ok(uri)
+ Ok(self.uri_storage.token_uri(
+ token_id,
+ &self.erc721,
+ &self.metadata,
+ )?)
  }
 
  #[selector(name = "setTokenURI")]