Bug/93 update vec to bounded vec

[bmacer]

addresses #93. We may be able to alias some of the generics to make it a bit more readable, but this should pass tests for all of equip, market and core, and should successfully remove #[pallet::without_storage_info]

[ilionic]

Great stuff, few minor comments.

[HashWarlock]

No longer needed comments can be removed and one request on the burn_nft if it is plausible.

[HashWarlock]

Remove unused code. I still think the drain_prefix should be used for the burn_nft function. Maybe @h4x3rotab can clarify if my suggestion is optimal in this situation. Overall looks good though just a few minor changes.

[bmacer]

Remove unused code. I still think the drain_prefix should be used for the burn_nft function. Maybe @h4x3rotab can clarify if my suggestion is optimal in this situation. Overall looks good though just a few minor changes.

@HashWarlock fixed in 390f3c2
Couldn't add to burn_nft, because we need to iterate through in order to remove from the Children storage. but I implemented the drain_prefix inside that loop. Not sure if that's efficient enough, but that seems required, since we need to iterate through the Children in order to burn, for example, grandchildren.

Cleaned up all the dead code mentioned.

[HashWarlock]

Remove unused code. I still think the drain_prefix should be used for the burn_nft function. Maybe @h4x3rotab can clarify if my suggestion is optimal in this situation. Overall looks good though just a few minor changes.

@HashWarlock fixed in 390f3c2 Couldn't add to burn_nft, because we need to iterate through in order to remove from the Children storage. but I implemented the drain_prefix inside that loop. Not sure if that's efficient enough, but that seems required, since we need to iterate through the Children in order to burn, for example, grandchildren.

Cleaned up all the dead code mentioned.

The drain_prefix iterates though so there's no need for the iter_prefix. At least this is what I see from the docs https://crates.parity.io/frame_support/storage/types/struct.StorageDoubleMap.html#method.drain_prefix

[bmacer]

Remove unused code. I still think the drain_prefix should be used for the burn_nft function. Maybe @h4x3rotab can clarify if my suggestion is optimal in this situation. Overall looks good though just a few minor changes.

@HashWarlock fixed in 390f3c2 Couldn't add to burn_nft, because we need to iterate through in order to remove from the Children storage. but I implemented the drain_prefix inside that loop. Not sure if that's efficient enough, but that seems required, since we need to iterate through the Children in order to burn, for example, grandchildren.
Cleaned up all the dead code mentioned.

The drain_prefix iterates though so there's no need for the iter_prefix. At least this is what I see from the docs https://crates.parity.io/frame_support/storage/types/struct.StorageDoubleMap.html#method.drain_prefix

Right, but We need to iterate through the Children to get the Nfts to burn. We can use drain_prefix inside of this (and outside of it), but I don't see how to fully eliminate iter_prefix since we're using the data we're iterating through to drain other storages. Maybe I'm missing something?

rmrk-substrate/pallets/rmrk-core/src/functions.rs

Lines 317 to 331 in ead0e1e

	Nfts::<T>::remove(collection_id, nft_id);
	for _ in Resources::<T>::drain_prefix((collection_id, nft_id)) {}
	for ((child_collection_id, child_nft_id), _) in Children::<T>::iter_prefix((collection_id, nft_id,)) {
	for _ in Children::<T>::drain_prefix((collection_id, nft_id)) {}
	Self::nft_burn(child_collection_id, child_nft_id, max_recursions - 1)?;
	}
	// decrement nfts counter
	Collections::<T>::try_mutate(collection_id, |collection| -> DispatchResult {
	let collection = collection.as_mut().ok_or(Error::<T>::CollectionUnknown)?;
	collection.nfts_count.saturating_dec();
	Ok(())
	})?;

[HashWarlock]

Remove unused code. I still think the drain_prefix should be used for the burn_nft function. Maybe @h4x3rotab can clarify if my suggestion is optimal in this situation. Overall looks good though just a few minor changes.

@HashWarlock fixed in 390f3c2 Couldn't add to burn_nft, because we need to iterate through in order to remove from the Children storage. but I implemented the drain_prefix inside that loop. Not sure if that's efficient enough, but that seems required, since we need to iterate through the Children in order to burn, for example, grandchildren.
Cleaned up all the dead code mentioned.

The drain_prefix iterates though so there's no need for the iter_prefix. At least this is what I see from the docs https://crates.parity.io/frame_support/storage/types/struct.StorageDoubleMap.html#method.drain_prefix

Right, but We need to iterate through the Children to get the Nfts to burn. We can use drain_prefix inside of this (and outside of it), but I don't see how to fully eliminate iter_prefix since we're using the data we're iterating through to drain other storages. Maybe I'm missing something?

rmrk-substrate/pallets/rmrk-core/src/functions.rs

Lines 317 to 331 in ead0e1e

	Nfts::<T>::remove(collection_id, nft_id);
	for _ in Resources::<T>::drain_prefix((collection_id, nft_id)) {}
	for ((child_collection_id, child_nft_id), _) in Children::<T>::iter_prefix((collection_id, nft_id,)) {
	for _ in Children::<T>::drain_prefix((collection_id, nft_id)) {}
	Self::nft_burn(child_collection_id, child_nft_id, max_recursions - 1)?;
	}
	// decrement nfts counter
	Collections::<T>::try_mutate(collection_id, |collection| -> DispatchResult {
	let collection = collection.as_mut().ok_or(Error::<T>::CollectionUnknown)?;
	collection.nfts_count.saturating_dec();
	Ok(())
	})?;

Maybe I'm confused. I was thinking this would be fine since we would get the child NFTs as we iterate the Children using drain_prefix. Then we call the burn_nft to take care of burning the children and their children as well. Does this cause problems with altering other storages?

 for ((child_collection_id, child_nft_id), _) in Children::<T>::drain_prefix((collection_id, nft_id,)) {  
 	Self::nft_burn(child_collection_id, child_nft_id, max_recursions - 1)?; 
 } 

[HashWarlock]

I spoke with @h4x3rotab and the suggestion to replace iter_prefix with drain_prefix should suffice since we will be recursively calling the burn_nft function with the child NFT's collection id and nft id. He also noted that whenever we call drain_prefix without utilizing the Iterator that is returned then we can instead call remove_prefix.

Here is an example of this on line 319:

rmrk-substrate/pallets/rmrk-core/src/functions.rs

Line 319 in ead0e1e

for _ in Resources::<T>::drain_prefix((collection_id, nft_id)) {}

Instead we can change it to this and avoid iterating through the Resources:

Resources::<T>::remove_prefix((collection_id, nft_id), None);

[ilionic]

Will merge to avoid conflicts, if any adjustments required please create new issue.