Burn NFTs causes panic

[HashWarlock]

Description

If the NestingBudget is set too low & there is a call to burn_nft, the function will panic! instead of throw an Error.

https://github.com/rmrk-team/rmrk-substrate/blob/c65927c8440c53f17e4bc55a5f28289d379f41aa/pallets/rmrk-core/src/functions.rs#L591-L594

Is there a reason we are subtracting from the Budget after the Storage mutation & event emitted? Also, we should use a safer operation to prevent the overflow like saturating_sub.

[ilionic]

@HashWarlock which values for NestingBudget did you use? Pushed hotfix update to use saturating_sub https://github.com/rmrk-team/rmrk-substrate/commit/f2191fda8c6e7e79c154fa46791e4c1f37e8d86e

Is there a reason we are subtracting from the Budget after the Storage mutation & event emitted?

budget itself subtracted earlier in budget.consume(). This part is for benchmarking and weight info.

[HashWarlock]

@HashWarlock which values for NestingBudget did you use? Pushed hotfix update to use saturating_sub f2191fd

Is there a reason we are subtracting from the Budget after the Storage mutation & event emitted?

budget itself subtracted earlier in budget.consume(). This part is for benchmarking and weight info.

The error came from doing a runtime upgrade and having a lower value for max iteration to the Budget version that was set to 20 vs 1_000. Either way, I think we should avoid doing any calculations after we have mutated storage and emitted an event. This calculation should happen earlier & return an error if an overflow is detected.

[HashWarlock]

I also think we need to prevent users from sending nested NFT if it were to be greater than the budget allowed for nesting.