diff --git a/Cargo.lock b/Cargo.lock index 2009360d..d13c4c9f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6719,6 +6719,7 @@ dependencies = [ "parity-scale-codec", "scale-info", "serde", + "sp-arithmetic", "sp-core", "sp-io", "sp-runtime", diff --git a/pallets/living-assets-ownership/Cargo.toml b/pallets/living-assets-ownership/Cargo.toml index 9b4a4f32..85ff1af9 100644 --- a/pallets/living-assets-ownership/Cargo.toml +++ b/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 } diff --git a/pallets/living-assets-ownership/src/functions.rs b/pallets/living-assets-ownership/src/functions.rs index ab3dce53..1e5beb4c 100644 --- a/pallets/living-assets-ownership/src/functions.rs +++ b/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 Pallet { - /// See [Self::create_collection] - pub fn do_create_collection( - collection_id: T::CollectionId, - who: T::AccountId, - ) -> DispatchResult { - ensure!( - !OwnerOfCollection::::contains_key(collection_id), - Error::::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::::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::::CollectionIdOverflow)?; + CollectionCounter::::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(()) } } diff --git a/pallets/living-assets-ownership/src/lib.rs b/pallets/living-assets-ownership/src/lib.rs index d5219cfb..bc0c7d5c 100644 --- a/pallets/living-assets-ownership/src/lib.rs +++ b/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: /// 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(_); @@ -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> + IsType<::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 = StorageMap<_, Blake2_128Concat, T::CollectionId, T::AccountId, OptionQuery>; + /// Collection counter + #[pallet::storage] + #[pallet::getter(fn collection_counter)] + pub(super) type CollectionCounter = 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 { /// 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 Pallet { #[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, - collection_id: T::CollectionId, - ) -> DispatchResult { + pub fn create_collection(origin: OriginFor) -> 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`: 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 { - /// Get owner of collection - fn owner_of_collection(collection_id: CollectionId) -> Option; - - /// Create collection - fn create_collection(collection_id: CollectionId, who: AccountId) -> DispatchResult; +impl traits::CollectionManager for Pallet { + fn owner_of_collection(collection_id: T::CollectionId) -> Option { + OwnerOfCollection::::get(collection_id) } - impl LivingAssetsOwnership for Pallet { - fn owner_of_collection(collection_id: T::CollectionId) -> Option { - OwnerOfCollection::::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) } } diff --git a/pallets/living-assets-ownership/src/tests.rs b/pallets/living-assets-ownership/src/tests.rs index 0251eb82..af17cb7f 100644 --- a/pallets/living-assets-ownership/src/tests.rs +++ b/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::::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 = >::create_collection(0, 1); + let result = >::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!(>::owner_of_collection(0), None); - assert_eq!(>::owner_of_collection(1), None); - }); - } - - #[test] - fn living_assets_ownership_trait_create_an_existing_collection_should_fail() { - new_test_ext().execute_with(|| { - assert_ok!(>::create_collection(0, 1)); - assert_noop!( - >::create_collection(0, 1), - Error::::CollectionAlreadyExists - ); + assert_eq!(>::owner_of_collection(0), None); + assert_eq!(>::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!(>::create_collection(0, 1)); + assert_ok!(>::create_collection( 1)); System::assert_last_event(Event::CollectionCreated { collection_id: 0, who: 1 }.into()); }); } diff --git a/pallets/living-assets-ownership/src/traits.rs b/pallets/living-assets-ownership/src/traits.rs new file mode 100644 index 00000000..edd9a59a --- /dev/null +++ b/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`: 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 { + /// Get owner of collection + fn owner_of_collection(collection_id: CollectionId) -> Option; + + /// Create collection + fn create_collection(who: AccountId) -> DispatchResult; +} diff --git a/precompile/living-assets/contracts/LivingAssetsOwnership.sol b/precompile/living-assets/contracts/LivingAssetsOwnership.sol index fb8f65ef..8bac6610 100644 --- a/precompile/living-assets/contracts/LivingAssetsOwnership.sol +++ b/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 diff --git a/precompile/living-assets/contracts/LivingAssetsOwnershipWrapper.sol b/precompile/living-assets/contracts/LivingAssetsOwnershipWrapper.sol deleted file mode 100644 index eab17502..00000000 --- a/precompile/living-assets/contracts/LivingAssetsOwnershipWrapper.sol +++ /dev/null @@ -1,23 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0-only -pragma solidity >=0.8.3; -import "./LivingAssetsOwnership.sol"; - -contract A { - LivingAssets public constant LIVING_ASSETS = - LivingAssets(0x0000000000000000000000000000000000000402); - - /// @notice Get owner of collection - function ownerOfCollection( - uint64 collection_id - ) public view returns (bytes32) { - return LIVING_ASSETS.ownerOfCollection(collection_id); - } - - /// @notice Create collection - function createCollection( - uint64 collection_id, - address who - ) public payable { - LIVING_ASSETS.createCollection(collection_id, who); - } -} diff --git a/precompile/living-assets/src/lib.rs b/precompile/living-assets/src/lib.rs index 91509c38..6148da90 100644 --- a/precompile/living-assets/src/lib.rs +++ b/precompile/living-assets/src/lib.rs @@ -3,7 +3,7 @@ #![cfg_attr(not(feature = "std"), no_std)] #![cfg_attr(test, feature(assert_matches))] use fp_evm::{ExitError, ExitSucceed, PrecompileFailure, PrecompileHandle, PrecompileOutput}; -use pallet_living_assets_ownership::LivingAssetsOwnership; +use pallet_living_assets_ownership::traits::CollectionManager; use parity_scale_codec::Encode; use precompile_utils::{ succeed, Address, EvmDataWriter, EvmResult, FunctionModifier, PrecompileHandleExt, @@ -17,7 +17,7 @@ use sp_std::{fmt::Debug, marker::PhantomData}; #[derive(Debug, PartialEq)] pub enum Action { /// Create a new collection - CreateCollection = "createCollection(uint64,address)", + CreateCollection = "createCollection(address)", /// Get owner of the collection OwnerOfCollection = "ownerOfCollection(uint64)", } @@ -30,7 +30,7 @@ where AddressMapping: pallet_evm::AddressMapping, AccountId: Encode + Debug, CollectionId: BaseArithmetic + Debug, - LivingAssets: LivingAssetsOwnership; + LivingAssets: CollectionManager; impl LivingAssetsOwnershipPrecompile @@ -38,7 +38,7 @@ where AddressMapping: pallet_evm::AddressMapping, AccountId: Encode + Debug, CollectionId: BaseArithmetic + Debug, - LivingAssets: LivingAssetsOwnership, + LivingAssets: CollectionManager, { #[allow(clippy::new_without_default)] pub fn new() -> Self { @@ -52,7 +52,7 @@ where AddressMapping: pallet_evm::AddressMapping, AccountId: Encode + Debug, CollectionId: BaseArithmetic + Debug, - LivingAssets: LivingAssetsOwnership, + LivingAssets: CollectionManager, { fn execute(handle: &mut impl PrecompileHandle) -> EvmResult { let selector = handle.read_selector()?; @@ -85,12 +85,11 @@ where // write storage Action::CreateCollection => { let mut input = handle.read_input()?; - input.expect_arguments(2)?; + input.expect_arguments(1)?; - let collection_id = input.read::()?.saturated_into(); let owner = AddressMapping::into_account_id(input.read::
()?.0); - if LivingAssets::create_collection(collection_id, owner).is_err() { + if LivingAssets::create_collection(owner).is_err() { return Err(PrecompileFailure::Error { exit_status: ExitError::Other(sp_std::borrow::Cow::Borrowed( "Could net create collection",