From 2d9b3a1326a8bc1296a82eedd6067715eea733e3 Mon Sep 17 00:00:00 2001 From: Jegor Sidorenko Date: Mon, 29 May 2023 14:23:57 +0200 Subject: [PATCH 1/4] Add mint price to the witness object on mint and confirm it --- frame/nfts/src/lib.rs | 15 +++++++++++---- frame/nfts/src/tests.rs | 42 +++++++++++++++++++++++++++++++++++------ frame/nfts/src/types.rs | 6 ++++-- 3 files changed, 51 insertions(+), 12 deletions(-) diff --git a/frame/nfts/src/lib.rs b/frame/nfts/src/lib.rs index 1a068b95e2a73..9f33ee4b65172 100644 --- a/frame/nfts/src/lib.rs +++ b/frame/nfts/src/lib.rs @@ -560,6 +560,8 @@ pub mod pallet { WrongOwner, /// The witness data given does not match the current state of the chain. BadWitness, + /// The witness data should be provided. + WitnessRequired, /// Collection ID is already taken. CollectionIdInUse, /// Items within that collection are non-transferable. @@ -771,7 +773,8 @@ pub mod pallet { /// - `item`: An identifier of the new item. /// - `mint_to`: Account into which the item will be minted. /// - `witness_data`: When the mint type is `HolderOf(collection_id)`, then the owned - /// item_id from that collection needs to be provided within the witness data object. + /// item_id from that collection needs to be provided within the witness data object. If + /// the mint price is set, the it should be additionally confirmed in the `witness_data`. /// /// Note: the deposit will be taken from the `origin` and not the `owner` of the `item`. /// @@ -785,7 +788,7 @@ pub mod pallet { collection: T::CollectionId, item: T::ItemId, mint_to: AccountIdLookupOf, - witness_data: Option>, + witness_data: Option>>, ) -> DispatchResult { let caller = ensure_signed(origin)?; let mint_to = T::Lookup::lookup(mint_to)?; @@ -817,8 +820,8 @@ pub mod pallet { ); }, MintType::HolderOf(collection_id) => { - let MintWitness { owned_item } = - witness_data.ok_or(Error::::BadWitness)?; + let MintWitness { owned_item, .. } = + witness_data.clone().ok_or(Error::::WitnessRequired)?; let owns_item = Account::::contains_key(( &caller, @@ -858,6 +861,10 @@ pub mod pallet { } if let Some(price) = mint_settings.price { + let MintWitness { mint_price, .. } = + witness_data.clone().ok_or(Error::::WitnessRequired)?; + let mint_price = mint_price.ok_or(Error::::BadWitness)?; + ensure!(mint_price >= price, Error::::BadWitness); T::Currency::transfer( &caller, &collection_details.owner, diff --git a/frame/nfts/src/tests.rs b/frame/nfts/src/tests.rs index 55bcf57e8aaff..4df57cb13218f 100644 --- a/frame/nfts/src/tests.rs +++ b/frame/nfts/src/tests.rs @@ -369,7 +369,37 @@ fn mint_should_work() { MintSettings { mint_type: MintType::Public, price: Some(1), ..Default::default() } )); Balances::make_free_balance_be(&account(2), 100); - assert_ok!(Nfts::mint(RuntimeOrigin::signed(account(2)), 0, 43, account(2), None)); + assert_noop!( + Nfts::mint(RuntimeOrigin::signed(account(2)), 0, 43, account(2), None,), + Error::::WitnessRequired + ); + assert_noop!( + Nfts::mint( + RuntimeOrigin::signed(account(2)), + 0, + 43, + account(2), + Some(MintWitness { ..Default::default() }) + ), + Error::::BadWitness + ); + assert_noop!( + Nfts::mint( + RuntimeOrigin::signed(account(2)), + 0, + 43, + account(2), + Some(MintWitness { mint_price: Some(0), ..Default::default() }) + ), + Error::::BadWitness + ); + assert_ok!(Nfts::mint( + RuntimeOrigin::signed(account(2)), + 0, + 43, + account(2), + Some(MintWitness { mint_price: Some(1), ..Default::default() }) + )); assert_eq!(Balances::total_balance(&account(2)), 99); // validate types @@ -385,11 +415,11 @@ fn mint_should_work() { )); assert_noop!( Nfts::mint(RuntimeOrigin::signed(account(3)), 1, 42, account(3), None), - Error::::BadWitness + Error::::WitnessRequired ); assert_noop!( Nfts::mint(RuntimeOrigin::signed(account(2)), 1, 42, account(2), None), - Error::::BadWitness + Error::::WitnessRequired ); assert_noop!( Nfts::mint( @@ -397,7 +427,7 @@ fn mint_should_work() { 1, 42, account(2), - Some(MintWitness { owned_item: 42 }) + Some(MintWitness { owned_item: 42, ..Default::default() }) ), Error::::BadWitness ); @@ -406,7 +436,7 @@ fn mint_should_work() { 1, 42, account(2), - Some(MintWitness { owned_item: 43 }) + Some(MintWitness { owned_item: 43, ..Default::default() }) )); // can't mint twice @@ -416,7 +446,7 @@ fn mint_should_work() { 1, 46, account(2), - Some(MintWitness { owned_item: 43 }) + Some(MintWitness { owned_item: 43, ..Default::default() }) ), Error::::AlreadyClaimed ); diff --git a/frame/nfts/src/types.rs b/frame/nfts/src/types.rs index bea1eca1f4832..fe648792237c5 100644 --- a/frame/nfts/src/types.rs +++ b/frame/nfts/src/types.rs @@ -124,10 +124,12 @@ impl CollectionDetails { } /// Witness data for items mint transactions. -#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, TypeInfo)] -pub struct MintWitness { +#[derive(Clone, Encode, Decode, Default, Eq, PartialEq, RuntimeDebug, TypeInfo)] +pub struct MintWitness { /// Provide the id of the item in a required collection. pub owned_item: ItemId, + /// The price specified in mint settings. + pub mint_price: Option, } /// Information concerning the ownership of a single unique item. From aee4bffa95724d13c0b70a4f160379973e23a6f8 Mon Sep 17 00:00:00 2001 From: Jegor Sidorenko Date: Mon, 29 May 2023 15:18:16 +0200 Subject: [PATCH 2/4] Chore --- frame/nft-fractionalization/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nft-fractionalization/src/tests.rs b/frame/nft-fractionalization/src/tests.rs index 8564b80533e0d..f9816460d2edf 100644 --- a/frame/nft-fractionalization/src/tests.rs +++ b/frame/nft-fractionalization/src/tests.rs @@ -105,7 +105,7 @@ fn fractionalize_should_work() { ), DispatchError::Module(ModuleError { index: 4, - error: [12, 0, 0, 0], + error: [13, 0, 0, 0], message: Some("ItemLocked") }) ); From d38fb57ad21366ca3facc02522321513752721e7 Mon Sep 17 00:00:00 2001 From: Jegor Sidorenko Date: Mon, 29 May 2023 16:36:02 +0200 Subject: [PATCH 3/4] Put the new error to the bottom --- frame/nft-fractionalization/src/tests.rs | 2 +- frame/nfts/src/lib.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/nft-fractionalization/src/tests.rs b/frame/nft-fractionalization/src/tests.rs index f9816460d2edf..8564b80533e0d 100644 --- a/frame/nft-fractionalization/src/tests.rs +++ b/frame/nft-fractionalization/src/tests.rs @@ -105,7 +105,7 @@ fn fractionalize_should_work() { ), DispatchError::Module(ModuleError { index: 4, - error: [13, 0, 0, 0], + error: [12, 0, 0, 0], message: Some("ItemLocked") }) ); diff --git a/frame/nfts/src/lib.rs b/frame/nfts/src/lib.rs index 9f33ee4b65172..fc2d30ab08550 100644 --- a/frame/nfts/src/lib.rs +++ b/frame/nfts/src/lib.rs @@ -560,8 +560,6 @@ pub mod pallet { WrongOwner, /// The witness data given does not match the current state of the chain. BadWitness, - /// The witness data should be provided. - WitnessRequired, /// Collection ID is already taken. CollectionIdInUse, /// Items within that collection are non-transferable. @@ -638,6 +636,8 @@ pub mod pallet { WrongNamespace, /// Can't delete non-empty collections. CollectionNotEmpty, + /// The witness data should be provided. + WitnessRequired, } #[pallet::call] From 0ce16590c5fd30bf234f1c63e6f70442598cc453 Mon Sep 17 00:00:00 2001 From: Jegor Sidorenko <5252494+jsidorenko@users.noreply.github.com> Date: Mon, 29 May 2023 17:45:21 +0200 Subject: [PATCH 4/4] Update frame/nfts/src/lib.rs Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> --- frame/nfts/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nfts/src/lib.rs b/frame/nfts/src/lib.rs index fc2d30ab08550..052a017f67092 100644 --- a/frame/nfts/src/lib.rs +++ b/frame/nfts/src/lib.rs @@ -774,7 +774,7 @@ pub mod pallet { /// - `mint_to`: Account into which the item will be minted. /// - `witness_data`: When the mint type is `HolderOf(collection_id)`, then the owned /// item_id from that collection needs to be provided within the witness data object. If - /// the mint price is set, the it should be additionally confirmed in the `witness_data`. + /// the mint price is set, then it should be additionally confirmed in the `witness_data`. /// /// Note: the deposit will be taken from the `origin` and not the `owner` of the `item`. ///