diff --git a/pallets/living-assets-ownership/src/functions.rs b/pallets/living-assets-ownership/src/functions.rs index abf2b028..9065d9cf 100644 --- a/pallets/living-assets-ownership/src/functions.rs +++ b/pallets/living-assets-ownership/src/functions.rs @@ -5,7 +5,7 @@ use sp_core::{H160, U256}; impl Pallet { /// See [Self::create_collection] - pub fn do_create_collection(who: T::AccountId) -> Result { + pub fn do_create_collection(who: T::AccountId) -> Result> { // Retrieve the current collection count to use as the new collection's ID let collection_id = Self::collection_counter(); diff --git a/pallets/living-assets-ownership/src/lib.rs b/pallets/living-assets-ownership/src/lib.rs index 57cfc62a..23e5c39a 100644 --- a/pallets/living-assets-ownership/src/lib.rs +++ b/pallets/living-assets-ownership/src/lib.rs @@ -55,8 +55,6 @@ pub mod pallet { // Errors inform users that something went wrong. #[pallet::error] pub enum Error { - /// Collection already exists - CollectionAlreadyExists, /// Collection id overflow CollectionIdOverflow, } @@ -83,16 +81,29 @@ pub mod pallet { OwnerOfCollection::::get(collection_id) } - fn create_collection(owner: T::AccountId) -> Result { - Self::do_create_collection(owner) + fn create_collection( + owner: T::AccountId, + ) -> Result { + match Self::do_create_collection(owner) { + Ok(collection_id) => Ok(collection_id), + Err(err) => match err { + Error::CollectionIdOverflow => { + Err(traits::CollectionManagerError::CollectionIdOverflow) + }, + _ => Err(traits::CollectionManagerError::UnknownError), + }, + } } } impl traits::Erc721 for Pallet { - fn owner_of(collection_id: CollectionId, asset_id: U256) -> Result { + fn owner_of( + collection_id: CollectionId, + asset_id: U256, + ) -> Result { match OwnerOfCollection::::get(collection_id) { Some(_) => Ok(convert_asset_id_to_owner(asset_id)), - None => Err("Collection does not exist"), + None => Err(traits::Erc721Error::UnexistentCollection), } } } diff --git a/pallets/living-assets-ownership/src/tests.rs b/pallets/living-assets-ownership/src/tests.rs index a6e34157..824f3ba7 100644 --- a/pallets/living-assets-ownership/src/tests.rs +++ b/pallets/living-assets-ownership/src/tests.rs @@ -3,10 +3,10 @@ use core::str::FromStr; use crate::{ address_to_collection_id, collection_id_to_address, is_collection_address, mock::*, - traits::{CollectionManager, Erc721}, + traits::{self, CollectionManager, Erc721}, CollectionError, Event, }; -use frame_support::assert_ok; +use frame_support::{assert_err, assert_ok}; use sp_core::H160; type AccountId = ::AccountId; @@ -119,10 +119,8 @@ fn living_assets_ownership_trait_id_of_new_collection_should_be_consecutive() { #[test] fn erc721_owner_of_asset_of_unexistent_collection() { new_test_ext().execute_with(|| { - assert_eq!( - ::owner_of(0, 2.into()), - Err("Collection does not exist") - ); + let result = ::owner_of(0, 2.into()); + assert_err!(result, traits::Erc721Error::UnexistentCollection); }); } diff --git a/pallets/living-assets-ownership/src/traits.rs b/pallets/living-assets-ownership/src/traits.rs index c5db2984..f958f8be 100644 --- a/pallets/living-assets-ownership/src/traits.rs +++ b/pallets/living-assets-ownership/src/traits.rs @@ -1,48 +1,94 @@ -use sp_core::{H160, U256}; - use crate::CollectionId; +use sp_core::{H160, U256}; -/// The `CollectionManager` trait provides an interface for managing collections in a -/// decentralized and non-fungible asset management system. This system allows for the creation of -/// collections, each of which can be owned by a unique `AccountId`. +/// The `CollectionManager` trait provides an interface for managing collections in a decentralized system. /// -/// A collection in this context can be thought of as a container for non-fungible assets. -/// Each collection has an associated `collection_id` which is a unique identifier for the collection -/// and can be used to retrieve the owner of the collection. +/// A collection is a container for non-fungible assets, uniquely identified by a `collection_id`. The system allows +/// for the creation and ownership management of these collections. /// /// # Methods /// -/// - `owner_of_collection(collection_id: CollectionId) -> Option`: This method retrieves the owner -/// of a collection given its `collection_id`. If no collection exists with the provided `collection_id`, -/// the method returns `None`. -/// -/// - `create_collection(owner: AccountId) -> Result`: This method creates a -/// new collection and assigns ownership to the provided `AccountId`. The method returns the `collection_id` -/// of the newly created collection. -/// +/// - `owner_of_collection`: Retrieve the owner of a specified collection. +/// - `create_collection`: Create a new collection and assign it to an owner. pub trait CollectionManager { - /// Get owner of collection + /// Retrieves the owner of the specified collection. + /// + /// # Arguments + /// + /// * `collection_id` - The unique identifier of the collection. + /// + /// # Returns + /// + /// The account ID of the collection's owner or `None` if the collection doesn't exist. fn owner_of_collection(collection_id: CollectionId) -> Option; - /// Create collection - fn create_collection(owner: AccountId) -> Result; + /// Creates a new collection and assigns it to the specified owner. + /// + /// # Arguments + /// + /// * `owner` - The account ID of the new collection's owner. + /// + /// # Returns + /// + /// A result containing the `collection_id` of the newly created collection or an error. + fn create_collection(owner: AccountId) -> Result; +} + +/// Errors that can occur when managing collections. +/// +/// - `CollectionAlreadyExists`: A collection with the same ID already exists. +/// - `CollectionIdOverflow`: The ID for the new collection would overflow. +/// - `UnknownError`: An unspecified error occurred. +#[derive(Debug, PartialEq)] +pub enum CollectionManagerError { + CollectionAlreadyExists, + CollectionIdOverflow, + UnknownError, } -/// `Erc721` Trait +impl AsRef<[u8]> for CollectionManagerError { + fn as_ref(&self) -> &[u8] { + match self { + CollectionManagerError::CollectionAlreadyExists => b"CollectionAlreadyExists", + CollectionManagerError::CollectionIdOverflow => b"CollectionIdOverflow", + CollectionManagerError::UnknownError => b"UnknownError", + } + } +} + +/// The `Erc721` trait provides an interface for handling ERC721 tokens in a blockchain environment. +/// +/// ERC721 tokens are a standard for representing ownership of unique items on the Ethereum blockchain. /// -/// This trait provides an interface for handling ERC721 tokens, a standard for non-fungible tokens on the blockchain. +/// # Methods +/// +/// - `owner_of`: Retrieve the owner of a specific asset within a collection. pub trait Erc721 { - /// Retrieves the owner of a specific asset in a collection. + /// Retrieves the owner of a specific asset within the specified collection. /// - /// # Parameters + /// # Arguments /// - /// * `collection_id`: An identifier for the collection to which the asset belongs. - /// * `asset_id`: The unique identifier for the asset within the specified collection. + /// * `collection_id` - The unique identifier for the collection. + /// * `asset_id` - The unique identifier for the asset within the collection. /// /// # Returns /// - /// * A `Result` which is: - /// - `Ok(H160)`: Returns the Ethereum address (`H160`) of the owner of the asset. - /// - `Err(&'static str)`: Returns an error message if the asset owner could not be determined. - fn owner_of(collection_id: CollectionId, asset_id: U256) -> Result; + /// The Ethereum address (`H160`) of the asset's owner or an error. + fn owner_of(collection_id: CollectionId, asset_id: U256) -> Result; +} + +/// Errors that can occur when interacting with ERC721 tokens. +/// +/// - `UnexistentCollection`: The specified collection does not exist. +#[derive(Debug, PartialEq)] +pub enum Erc721Error { + UnexistentCollection, +} + +impl AsRef<[u8]> for Erc721Error { + fn as_ref(&self) -> &[u8] { + match self { + Erc721Error::UnexistentCollection => b"UnexistentCollection", + } + } } diff --git a/precompile/erc721/src/tests.rs b/precompile/erc721/src/tests.rs index 07fc1b4a..b9a7b1e2 100644 --- a/precompile/erc721/src/tests.rs +++ b/precompile/erc721/src/tests.rs @@ -1,7 +1,7 @@ use core::str::FromStr; use super::*; -use pallet_living_assets_ownership::CollectionId; +use pallet_living_assets_ownership::{traits::Erc721Error, CollectionId}; use precompile_utils::testing::create_mock_handle_from_input; use sp_core::{H160, U256}; @@ -39,7 +39,7 @@ fn owner_of_asset_should_return_an_address() { #[test] fn if_mock_fails_should_return_the_error() { - impl_precompile_mock_simple!(Mock, Err("spaghetti error")); + impl_precompile_mock_simple!(Mock, Err(Erc721Error::UnexistentCollection)); let owner_of_asset_4 = hex::decode("6352211e0000000000000000000000000000000000000000000000000000000000000004") @@ -48,7 +48,7 @@ fn if_mock_fails_should_return_the_error() { handle.code_address = H160::from_str("ffffffffffffffffffffffff0000000000000005").unwrap(); let result = Mock::execute(&mut handle); assert!(result.is_err()); - assert_eq!(result.unwrap_err(), revert("spaghetti error"),); + assert_eq!(result.unwrap_err(), revert(Erc721Error::UnexistentCollection)); } #[test] @@ -91,7 +91,7 @@ mod helpers { fn owner_of( collectio_id: CollectionId, asset_id: U256, - ) -> Result { + ) -> Result { ($owner_of_collection)(collectio_id, asset_id) } } diff --git a/precompile/living-assets/src/tests.rs b/precompile/living-assets/src/tests.rs index a4ac1b8e..6d404a1d 100644 --- a/precompile/living-assets/src/tests.rs +++ b/precompile/living-assets/src/tests.rs @@ -4,6 +4,7 @@ #![allow(clippy::redundant_closure_call)] use super::*; +use pallet_living_assets_ownership::traits::CollectionManagerError; use precompile_utils::{ revert, succeed, testing::{create_mock_handle, create_mock_handle_from_input}, @@ -31,11 +32,15 @@ fn check_log_selectors() { #[test] fn failing_create_collection_should_return_error() { - impl_precompile_mock_simple!(Mock, Err("spaghetti code"), Some(H160::zero())); + impl_precompile_mock_simple!( + Mock, + Err(CollectionManagerError::CollectionAlreadyExists), + Some(H160::zero()) + ); let mut handle = create_mock_handle_from_input(hex::decode(CREATE_COLLECTION).unwrap()); let result = Mock::execute(&mut handle); - assert_eq!(result.unwrap_err(), revert("spaghetti code")); + assert_eq!(result.unwrap_err(), revert(CollectionManagerError::CollectionAlreadyExists)); } #[test] @@ -142,7 +147,9 @@ mod helpers { impl pallet_living_assets_ownership::traits::CollectionManager for CollectionManagerMock { - fn create_collection(owner: AccountId) -> Result { + fn create_collection( + owner: AccountId, + ) -> Result { ($create_collection_result)(owner) }