Pallet uniques: Mint with extra deposit #753
Conversation
Hounsette
commented
Jun 16, 2023
•
Hounsette
commented
- Add create collection with extra deposit limit
- Add update extra deposit limit for existing collections
- Add mint with extra deposit
- Unreserve the extra deposit on burning an item and transfer it to item owner
- Unreserve all extra deposits on destroying a collection and transfer them to item owners
- Repatriate the total extra deposit when the ownership of collection changes
- Benchmark nodle-uniques-pallet
- Relocate Uniques to SubstrateUniques pallet on runtime upgrade
… by allowing dynamic intrinsic value for NFTs We aim to cover all the main substrate uniques calls wrapped in our own Nodle uniques while keeping the name Uniques for this new module to minimize api change from client's point of view.
…we can easily implement light wraps of all the calls while extending that pallet
This reverts commit 207e955.
This reverts commit af06fcb.
…the collection owner
Codecov Report
@@ Coverage Diff @@
## master #753 +/- ##
==========================================
+ Coverage 58.81% 64.53% +5.71%
==========================================
Files 38 43 +5
Lines 4740 6116 +1376
==========================================
+ Hits 2788 3947 +1159
- Misses 1952 2169 +217
|
…ntime-benchmark, necessary only for visibility for tests
…uniques storage are recovered before being cleared
…unreserve is partial
… located at index 42
…ew functions [WIP]
…ith_extra_deposit_limit and transfer_ownership
Co-authored-by: Eliott Teissonniere <[email protected]>
|
|
||
| pub struct MovePalletUniquesToSubstrateUniques; | ||
| impl OnRuntimeUpgrade for MovePalletUniquesToSubstrateUniques { | ||
| fn on_runtime_upgrade() -> Weight { |
There was a problem hiding this comment.
How does this trigger...
- check trigger
- run and upgrade on local testnet
pallets/uniques/src/benchmarking.rs
Outdated
| @@ -0,0 +1,181 @@ | |||
| // This file is part of Substrate. | |||
There was a problem hiding this comment.
What file did we copy? Did not all the code change?
Co-authored-by: Fredrik Simonsson <[email protected]>
| (collection_id, collection_owner, collection_owner_lookup) | ||
| } | ||
|
|
||
| fn create_collection<T: Config<I>, I: 'static>( |
There was a problem hiding this comment.
- There is only one create collection function not two versions
| /// The total extra deposit reserved so far | ||
| /// | ||
| /// Ever mint with extra deposit should update this value. | ||
| total: T, |
There was a problem hiding this comment.
Do we need to store both total and limit one alternative could be remaining
There was a problem hiding this comment.
It would be useful to be able to query the total and limit separately. Otherwise for knowing how much is the total reserved for a collection we have to check and add all the items extra deposits through potentially several queries.
| let item_owner = | ||
| pallet_uniques::Pallet::<T, I>::owner(collection, item).ok_or(Error::<T, I>::UnknownItemOwner)?; | ||
| <T as pallet_uniques::Config<I>>::Currency::unreserve(&collection_owner, extra_deposit); | ||
| <T as pallet_uniques::Config<I>>::Currency::transfer( |
There was a problem hiding this comment.
I think extra deposit should be owned by owner as reserved tokens, shouldn't it?
There was a problem hiding this comment.
When there is no collection, there is no reason for keeping a fund reserved. This is the unreserve condition. Otherwise we may lock tokens forever
Plus benchmark update_extra_deposit_limit locally
