Make resource_id / nft_id required #205
Conversation
ilionic
changed the title
ilionic
changed the title
| if let Some(max) = collection.max { | ||
| ensure!(nft_id < max, Error::<T>::CollectionFullOrLocked); | ||
| ensure!(collection.nfts_count < max, Error::<T>::CollectionFullOrLocked); |
There was a problem hiding this comment.
Should we add a helper function that gets the Collection's NFT count?
There was a problem hiding this comment.
pub fn get_collection_nfts_count(collection_info: CollectionInfoOf<T>) -> u32 {
collection_info.nfts_count
}
There was a problem hiding this comment.
What are your thoughts? It's pretty simple, but we could not do this, too.
There was a problem hiding this comment.
I am not sure actually, seems a bit unneeded abstraction of just accessing property? But happy to update if you think worth pursuing. ( Usually I would extract a function with more logic )
|
Should we think about adding |
Do you mean using a 'stringy' version instead of u32? There was another point in favour of u32 - to keep it compatible with EVM. We also discussed id's strategy extensively with Parity Uniques devs and basically agreed to keep u32 ( just align with auto inc strategies across the board, hence there was recent update in Uniques. So we are pretty much in line now ) |
Fixes #169