Improve nesting

[Szegoo]

Closes: #223

[Szegoo]

Just realized that this may be a bit early for a review(that is why I deleted the previous message), will ping you when it will be ready :)

[Szegoo]

@ilionic This is ready for review, other than the burn_nft I couldn't find any other functions that have recursion. Are these the only ones?

[ilionic]

Thanks @Szegoo. That's correct, recursive calls are only related to burning and root owner lookup.
Also please check unit tests:

error[E0046]: not all trait items implemented, missing: `NestingBudget`
   --> pallets/rmrk-market/src/mock.rs:107:1
    |
107 | impl pallet_rmrk_core::Config for Test {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `NestingBudget` in implementation

[Szegoo]

I don't think we need MaxRecursions.

[HashWarlock]

Do we prevent this scenario when sending/minting NFTs to NFTs? If we are able to get a Budget from lookup_root_owner, could we prevent this scenario from happening?

[Szegoo]

If we are able to get a Budget from lookup_root_owner, could we prevent this scenario from happening?

Not sure I understand this totally correctly but did you mean that we should somehow be able to get the Budget that is needed to look up the owner?

Edit: Maybe the functions that deal with budget should make it possible(but optional) to define the budget. For example:

fn property_set(
	sender: T::AccountId,
	collection_id: CollectionId,
        maybe_nft_id: Option<NftId>,
	key: KeyLimitOf<T>,
	value: ValueLimitOf<T>,
        budget: Option<&dyn Budget> // <- we add this to the functions that deal with budget.
) -> DispatchResult {

If the function is called with None we simply use the NestingBudget.
When we add benchmarks the weight could be calculated based on the budget provided.

This would make it possible to look up the owner for any asset(of course it will cost more if the nft is deeply nested)

Edit: AFAIU Doing nft_mint_directly_to_nft will fail if the nft that we are trying to mint up on is too deeply nested. Sorry if I didn't understand you correctly. @HashWarlock

[HashWarlock]

LGTM