Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TransferHooks, Budget fix and upgrade to Polkadot v0.9.36 #261

Merged
merged 18 commits into from
Jan 9, 2023

Conversation

HashWarlock
Copy link
Contributor

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 TransferHooks to be implemented with 2 functions pre_check and post_transfer that default to true when 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.

Update pallet index macro for pallet functions

This is a small change introduced in polkadot-v0.9.36 to add the pallet index

Targets

  • Upgrade to polkadot-v0.9.36
  • implement TransferHooks trait with pre_check and post_transfer functions
  • Update integration tests
  • Fix minting/sending NFTs that exceed the threshold of NestingBudget::get() value.
  • Add fields names budget_consumed and budget_left` to track & through errors is an overflow is detected.
  • Add functions in Budget trait to get the 2 new fields.
  • Fix benchmarking errors from Traits being renamed.
  • Add pallet index for pallet functions.

Copy link
Contributor

@ilionic ilionic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
merged current main to polkadot-v0.9.29.
Let's merge this one so other benchmarking PRs could be updated as well.
Thank you!

@HashWarlock
Copy link
Contributor Author

LGTM merged current main to polkadot-v0.9.29. Let's merge this one so other benchmarking PRs could be updated as well. Thank you!

I need to fix an odd build error. I didn't see this building and running locally. I'll have update asap.

@ilionic
Copy link
Contributor

ilionic commented Jan 9, 2023

@HashWarlock do you think we can merge this one? So we can update other benchmarking PRs with API changes

@HashWarlock
Copy link
Contributor Author

@HashWarlock do you think we can merge this one? So we can update other benchmarking PRs with API changes

Good to merge now. Then we can update the rest

@ilionic ilionic merged commit 7edcd14 into rmrk-team:main Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants