Feature/create collection return address

pallets/living-assets-ownership/Cargo.toml

@@ -19,6 +19,7 @@ scale-info = { workspace = true, features = ["derive"] }
 frame-benchmarking = { workspace = true, optional = true }
 frame-support = { workspace = true }
 frame-system = { workspace = true }
+sp-arithmetic = { workspace = true }
 
 [dev-dependencies]
 serde = { workspace = true }

pallets/living-assets-ownership/src/functions.rs

@@ -1,22 +1,54 @@
 //! Contains helper and utility functions of the pallet
 use super::*;
-use frame_support::{ensure, sp_runtime::DispatchResult};
+use frame_support::sp_runtime::{
+	traits::{CheckedAdd, One},
+	DispatchResult,
+};
 
 impl<T: Config> Pallet<T> {
-	/// See [Self::create_collection]
-	pub fn do_create_collection(
-		collection_id: T::CollectionId,
-		who: T::AccountId,
-	) -> DispatchResult {
-		ensure!(
-			!OwnerOfCollection::<T>::contains_key(collection_id),
-			Error::<T>::CollectionAlreadyExists
-		);
+	/// Attempts to create a new collection
+	///
+	/// This function is intended to be used as the implementation for the
+	/// [Self::create_collection] public interface. It first retrieves the current
+	/// collection count to use as the new collection's ID. It then inserts a new
+	/// entry into the `OwnerOfCollection` map, mapping the new collection's ID to
+	/// the owner's account ID.
+	///
+	/// After this, the function attempts to increment the collection counter by 1.
+	/// If this operation would result in an overflow, the function returns early
+	/// with an [Error::CollectionIdOverflow].
+	///
+	/// Finally, if all operations were successful, the function emits a
+	/// [Event::CollectionCreated] event and returns `Ok(())`.
+	///
+	/// # Arguments
+	///
+	/// * `who` - The account ID of the new collection's owner.
+	///
+	/// # Return
+	///
+	/// Returns a [DispatchResult] indicating the outcome of the operation. If the
+	/// operation was successful, the function returns `Ok(())`. If the operation
+	/// was not successful, the function returns `Err(e)`, where `e` is the error
+	/// that occurred.
+	pub fn do_create_collection(who: T::AccountId) -> DispatchResult {
+		// Retrieve the current collection count to use as the new collection's ID
+		let collection_id = Self::collection_counter();
 
+		// Insert a new entry into the OwnerOfCollection map, mapping the new
+		// collection's ID to the owner's account ID
 		OwnerOfCollection::<T>::insert(collection_id, &who);
 
+		// Attempt to increment the collection counter by 1. If this operation
+		// would result in an overflow, return early with an error
+		let counter =
+			collection_id.checked_add(&One::one()).ok_or(Error::<T>::CollectionIdOverflow)?;
+		CollectionCounter::<T>::put(counter);
+
+		// Emit an event indicating that a new collection was created
 		Self::deposit_event(Event::CollectionCreated { collection_id, who });
 
+		// Return Ok to indicate that the operation was successful
 		Ok(())
 	}
 }

pallets/living-assets-ownership/src/lib.rs

@@ -1,11 +1,13 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 
+use frame_support::dispatch::DispatchResult;
 /// Edit this file to define custom logic or remove it if it is not needed.
 /// Learn more about FRAME and the core library of Substrate FRAME pallets:
 /// <https://docs.substrate.io/reference/frame-pallets/>
 pub use pallet::*;
 
 mod functions;
+pub mod traits;
 
 #[cfg(test)]
 mod mock;
@@ -15,8 +17,12 @@ mod tests;
 
 #[frame_support::pallet]
 pub mod pallet {
-	use frame_support::pallet_prelude::{OptionQuery, *};
+	use frame_support::{
+		pallet_prelude::{OptionQuery, ValueQuery, *},
+		sp_runtime::traits::{CheckedAdd, One},
+	};
 	use frame_system::pallet_prelude::*;
+	use sp_arithmetic::traits::Unsigned;
 
 	#[pallet::pallet]
 	pub struct Pallet<T>(_);
@@ -27,7 +33,14 @@ pub mod pallet {
 		/// Because this pallet emits events, it depends on the runtime's definition of an event.
 		type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
 		/// Collection id type
-		type CollectionId: Member + Parameter + MaxEncodedLen + Copy;
+		type CollectionId: Member
+			+ Parameter
+			+ MaxEncodedLen
+			+ Copy
+			+ Default
+			+ CheckedAdd
+			+ One
+			+ Unsigned;
 	}
 
 	/// Mapping from collection id to owner
@@ -36,6 +49,11 @@ pub mod pallet {
 	pub(super) type OwnerOfCollection<T: Config> =
 		StorageMap<_, Blake2_128Concat, T::CollectionId, T::AccountId, OptionQuery>;
 
+	/// Collection counter
+	#[pallet::storage]
+	#[pallet::getter(fn collection_counter)]
+	pub(super) type CollectionCounter<T: Config> = StorageValue<_, T::CollectionId, ValueQuery>;
+
 	/// Pallet events
 	#[pallet::event]
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
@@ -50,6 +68,8 @@ pub mod pallet {
 	pub enum Error<T> {
 		/// Collection already exists
 		CollectionAlreadyExists,
+		/// Collection id overflow
+		CollectionIdOverflow,
 	}
 
 	// Dispatchable functions allows users to interact with the pallet and invoke state changes.
@@ -59,53 +79,19 @@ pub mod pallet {
 	impl<T: Config> Pallet<T> {
 		#[pallet::call_index(0)]
 		#[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] // TODO set proper weight
-		pub fn create_collection(
-			origin: OriginFor<T>,
-			collection_id: T::CollectionId,
-		) -> DispatchResult {
+		pub fn create_collection(origin: OriginFor<T>) -> DispatchResult {
 			let who = ensure_signed(origin)?;
-			Self::do_create_collection(collection_id, who)
+			Self::do_create_collection(who)
 		}
 	}
+}
 
-	/// The `LivingAssetsOwnership` 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`.
-	///
-	/// 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.
-	///
-	/// # Methods
-	///
-	/// - `owner_of_collection(collection_id: T::CollectionId) -> Option<AccountId>`: 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(collection_id: T::CollectionId, who: AccountId) -> DispatchResult`: This method creates a
-	/// new collection with the specified `collection_id` and assigns ownership to the provided `AccountId`.
-	/// If a collection already exists with the provided `collection_id`, the method will return an error.
-	///
-	/// # Errors
-	///
-	/// - `CollectionAlreadyExists`: This error is returned by the `create_collection` method when a collection
-	/// with the provided `collection_id` already exists.
-	///
-	pub trait LivingAssetsOwnership<AccountId, CollectionId> {
-		/// Get owner of collection
-		fn owner_of_collection(collection_id: CollectionId) -> Option<AccountId>;
-
-		/// Create collection
-		fn create_collection(collection_id: CollectionId, who: AccountId) -> DispatchResult;
+impl<T: Config> traits::CollectionManager<T::AccountId, T::CollectionId> for Pallet<T> {
+	fn owner_of_collection(collection_id: T::CollectionId) -> Option<T::AccountId> {
+		OwnerOfCollection::<T>::get(collection_id)
 	}
 
-	impl<T: Config> LivingAssetsOwnership<T::AccountId, T::CollectionId> for Pallet<T> {
-		fn owner_of_collection(collection_id: T::CollectionId) -> Option<T::AccountId> {
-			OwnerOfCollection::<T>::get(collection_id)
-		}
-
-		fn create_collection(collection_id: T::CollectionId, who: T::AccountId) -> DispatchResult {
-			Self::do_create_collection(collection_id, who)
-		}
+	fn create_collection(who: T::AccountId) -> DispatchResult {
+		Self::do_create_collection(who)
 	}
 }

pallets/living-assets-ownership/src/tests.rs

@@ -1,5 +1,5 @@
-use crate::{mock::*, Error, Event, LivingAssetsOwnership};
-use frame_support::{assert_noop, assert_ok};
+use crate::{mock::*, traits::CollectionManager, Event};
+use frame_support::assert_ok;
 
 #[cfg(test)]
 mod test {
@@ -19,37 +19,42 @@ mod test {
 	#[test]
 	fn create_new_collection() {
 		new_test_ext().execute_with(|| {
-			assert_ok!(LivingAssetsModule::create_collection(RuntimeOrigin::signed(1), 0));
+			assert_ok!(LivingAssetsModule::create_collection(RuntimeOrigin::signed(1)));
 			assert_eq!(LivingAssetsModule::owner_of_collection(0), Some(1));
 		});
 	}
 
 	#[test]
-	fn create_an_existing_collection_should_fail() {
+	fn create_new_collection_should_emit_an_event() {
 		new_test_ext().execute_with(|| {
-			assert_ok!(LivingAssetsModule::create_collection(RuntimeOrigin::signed(1), 0));
-			assert_noop!(
-				LivingAssetsModule::create_collection(RuntimeOrigin::signed(1), 0),
-				Error::<Test>::CollectionAlreadyExists
-			);
+			// Go past genesis block so events get deposited
+			System::set_block_number(1);
+
+			assert_ok!(LivingAssetsModule::create_collection(RuntimeOrigin::signed(1)));
+			System::assert_last_event(Event::CollectionCreated { collection_id: 0, who: 1 }.into());
 		});
 	}
 
 	#[test]
-	fn create_new_collection_should_emit_an_event() {
+	fn initially_collection_counter_is_0() {
 		new_test_ext().execute_with(|| {
-			// Go past genesis block so events get deposited
-			System::set_block_number(1);
+			assert_eq!(LivingAssetsModule::collection_counter(), 0);
+		});
+	}
 
-			assert_ok!(LivingAssetsModule::create_collection(RuntimeOrigin::signed(1), 0));
-			System::assert_last_event(Event::CollectionCreated { collection_id: 0, who: 1 }.into());
+	#[test]
+	fn create_collection_and_check_counter() {
+		new_test_ext().execute_with(|| {
+			assert_ok!(LivingAssetsModule::create_collection(RuntimeOrigin::signed(1)));
+			assert_eq!(LivingAssetsModule::collection_counter(), 1);
 		});
 	}
 
+	// Test CollectionManager trait
 	#[test]
 	fn living_assets_ownership_trait_create_new_collection_by_living() {
 		new_test_ext().execute_with(|| {
-        	let result = <LivingAssetsModule as LivingAssetsOwnership<AccountId, CollectionId>>::create_collection(0, 1);
+        	let result = <LivingAssetsModule as CollectionManager<AccountId, CollectionId>>::create_collection(1);
         	assert_ok!(result);
         	assert_eq!(LivingAssetsModule::owner_of_collection(0), Some(1));
     	});
@@ -58,19 +63,8 @@ mod test {
 	#[test]
 	fn living_assets_ownership_trait_owner_of_unexistent_collection_is_none() {
 		new_test_ext().execute_with(|| {
-			assert_eq!(<LivingAssetsModule as LivingAssetsOwnership<AccountId, CollectionId>>::owner_of_collection(0), None);
-			assert_eq!(<LivingAssetsModule as LivingAssetsOwnership<AccountId, CollectionId>>::owner_of_collection(1), None);
-		});
-	}
-
-	#[test]
-	fn living_assets_ownership_trait_create_an_existing_collection_should_fail() {
-		new_test_ext().execute_with(|| {
-			assert_ok!(<LivingAssetsModule as LivingAssetsOwnership<AccountId, CollectionId>>::create_collection(0, 1));
-			assert_noop!(
-				<LivingAssetsModule as LivingAssetsOwnership<AccountId, CollectionId>>::create_collection(0, 1),
-				Error::<Test>::CollectionAlreadyExists
-			);
+			assert_eq!(<LivingAssetsModule as CollectionManager<AccountId, CollectionId>>::owner_of_collection(0), None);
+			assert_eq!(<LivingAssetsModule as CollectionManager<AccountId, CollectionId>>::owner_of_collection(1), None);
 		});
 	}
 
@@ -80,7 +74,7 @@ mod test {
 			// Go past genesis block so events get deposited
 			System::set_block_number(1);
 
-			assert_ok!(<LivingAssetsModule as LivingAssetsOwnership<AccountId, CollectionId>>::create_collection(0, 1));
+			assert_ok!(<LivingAssetsModule as CollectionManager<AccountId, CollectionId>>::create_collection( 1));
 			System::assert_last_event(Event::CollectionCreated { collection_id: 0, who: 1 }.into());
 		});
 	}

pallets/living-assets-ownership/src/traits.rs

@@ -0,0 +1,32 @@
+use frame_support::dispatch::DispatchResult;
+
+/// 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`.
+///
+/// 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.
+///
+/// # Methods
+///
+/// - `owner_of_collection(collection_id: T::CollectionId) -> Option<AccountId>`: 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(collection_id: T::CollectionId, who: AccountId) -> DispatchResult`: This method creates a
+/// new collection with the specified `collection_id` and assigns ownership to the provided `AccountId`.
+/// If a collection already exists with the provided `collection_id`, the method will return an error.
+///
+/// # Errors
+///
+/// - `CollectionAlreadyExists`: This error is returned by the `create_collection` method when a collection
+/// with the provided `collection_id` already exists.
+///
+pub trait CollectionManager<AccountId, CollectionId> {
+	/// Get owner of collection
+	fn owner_of_collection(collection_id: CollectionId) -> Option<AccountId>;
+
+	/// Create collection
+	fn create_collection(who: AccountId) -> DispatchResult;
+}

precompile/living-assets/contracts/LivingAssetsOwnership.sol

@@ -13,7 +13,7 @@ interface LivingAssets {
     function createCollection(
         uint64 collection_id,
         address who
-    ) external payable;
+    ) external;
 
     /// @dev Get collection owner
     /// @custom:selector 0xfb34ae53