Add TransferHooks, Budget fix and upgrade to Polkadot v0.9.34 #259
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s for implementation downstream for custom logic
Upgrade to polkadot-v0.9.33 & Add checkAllowTransferFn Trait
…when Nesting child NFTs to a parent.
Fix budget issue with Nested NFTs
Upgrade to polkadot-v0.9.34 and fix benchmarking
7 tasks
Szegoo
reviewed
Jan 3, 2023
| /// the logic of RMRK's current funcitonality. | ||
| pub trait TransferHooks<AccountId, CollectionId, NftId> { | ||
| /// Check if the NFT's pre-checks and post-checks for the transfer function based on the sender, | ||
| /// collection_id and nft_id parameters. |
There was a problem hiding this comment.
Suggested change
| /// collection_id and nft_id parameters. | |
| /// `collection_id` and `nft_id` parameters. |
Szegoo
reviewed
Jan 3, 2023
| @@ -1229,12 +1229,22 @@ fn burn_nft_beyond_max_recursions_fails_gracefully() { | |||
| 3, | |||
| AccountIdOrCollectionNftTuple::CollectionAndNftTuple(0, 2), | |||
| )); | |||
| // ALICE sends NFT (0, 4) to NFT (0, 3) | |||
| // ALICE sends NFT (0, 4) to NFT (0, 3) fails bc there nested budget threshold is exceeded | |||
There was a problem hiding this comment.
Suggested change
| // ALICE sends NFT (0, 4) to NFT (0, 3) fails bc there nested budget threshold is exceeded | |
| // ALICE sends NFT (0, 4) to NFT (0, 3) fails bc the nested budget threshold is exceeded |
|
Closing in favor of #261 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
TransferHooks implementation
For implementations downstream of the RMRK pallet there needs to be an ability to add custom logic in the case that there are multiple pallets using the RMRK NFT pallet, but want to keep the business logic mutually exclusive. This implementation allows for a trait called
TransferHooksto be implemented with 2 functionspre_checkandpost_transferthat default totruewhen not implemented.Budget trait fix
There are 2 problems with the current Budget trait:
NFTs are currently allowed to mint and send NFTs to a parent NFT that can meet up to the threshold of NestingBudget::get() + 1. This allows for NFTs queries like lookup_root_owner to return errors like TooManyRecursions. Having this error is okay, but the NFTs should be prevented from being minted to or sent to a parent NFT if the Budget threshold is met.
Currently the code will do a calculation on functions like burn_nft and this will perform an arithmetic subtraction at the end of the function, but this should be added as a field in the Value type & implement a function from Budget to get the budget that has been consumed and budget that is left.
Targets
polkadot-v0.9.34TransferHookstrait withpre_checkandpost_transferfunctionsNestingBudget::get()value.budget_consumedand budget_left` to track & through errors is an overflow is detected.Budgettrait to get the 2 new fields.