Skip to content

Commit

Permalink
fix(L-02): Inconsistent Implementations in ERC-721 Extension #331 (#333)
Browse files Browse the repository at this point in the history 
Resolves #312

---------

Co-authored-by: Alisander Qoshqosh <[email protected]>
  • Loading branch information
@bidzyyys @qalisander
bidzyyys and qalisander authored Oct 17, 2024
1 parent b5b5c6b commit 47d2d9c
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 100 deletions.
89 changes: 59 additions & 30 deletions contracts/src/token/erc721/extensions/metadata.rs
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
//! 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::erc721::{Error, IErc721},
utils::{introspection::erc165::IErc165, Metadata},
};

sol_storage! {
/// Metadata of an [`crate::token::erc721::Erc721`] token.
Expand All @@ -19,7 +21,6 @@ sol_storage! {
}

/// Interface for the optional metadata functions from the ERC-721 standard.
#[interface_id]
pub trait IErc721Metadata {
/// Returns the token collection name.
///
Expand All @@ -34,14 +35,6 @@ pub trait IErc721Metadata {
///
/// * `&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.
Expand All @@ -56,29 +49,65 @@ impl IErc721Metadata for Erc721Metadata {
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)
}
}
119 changes: 103 additions & 16 deletions contracts/src/token/erc721/extensions/uri_storage.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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")
);
}
}
38 changes: 9 additions & 29 deletions examples/erc721-metadata/src/lib.rs
View file
Original file line number Diff line number Diff line change
@@ -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};

Expand All @@ -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)?)
Expand All @@ -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")]
Expand Down
1 change: 0 additions & 1 deletion examples/erc721-metadata/tests/abi/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ sol!(
function transferFrom(address from, address to, uint256 tokenId) external;
function mint(address to, uint256 tokenId) external;
function burn(uint256 tokenId) external;
function baseUri() external view returns (string memory baseURI);
function name() external view returns (string memory name);
function symbol() external view returns (string memory symbol);
#[derive(Debug)]
Expand Down
Loading

0 comments on commit 47d2d9c

Please sign in to comment.