Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Disallow burning externally locked nfts #13054

Merged
merged 2 commits into from
Jan 4, 2023
Merged

Disallow burning externally locked nfts #13054

merged 2 commits into from
Jan 4, 2023

Conversation

jsidorenko
Copy link
Contributor

If the NFT was locked externally via the Locker trait, the owner shouldn't be able to burn it.

@jsidorenko jsidorenko added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Jan 3, 2023
@jsidorenko
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 2967726 into master Jan 4, 2023
@paritytech-processbot paritytech-processbot bot deleted the js/improve-nft-locker branch January 4, 2023 08:30
@mustermeiszer
Copy link
Contributor

@jsidorenko One question: In the pallet-uniques the destroy extrinsic does not care whether an instance is locked, if I read it correctly.
Isn't this breaking a few assumptions about this locking here?

@mustermeiszer mustermeiszer mentioned this pull request Jan 18, 2023
Closed
@jsidorenko
Copy link
Contributor Author

Hi, according to the current design, the collection could be removed (destroyed) entirely by the owner regardless of the status of the items. In this PR I made it not possible to burn an item if it was externally locked by some other pallet (like a fractionalization pallet).
At the same time, you've raised a good question (especially in a created issue), I think we should continue discussing the permissions in that issue.

@AurevoirXavier AurevoirXavier mentioned this pull request Feb 8, 2023
Closed
@kacperzuk-neti kacperzuk-neti mentioned this pull request Feb 14, 2023
Merged
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
@jsidorenko @ltfschoen
Disallow burning externally locked nfts (paritytech#13054)
c53479b 
* Disallow burning externally locked nfts

* Update docs
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@jsidorenko @ark0f
Disallow burning externally locked nfts (paritytech#13054)
a9dbbdd 
* Disallow burning externally locked nfts

* Update docs
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gilescope gilescope gilescope approved these changes

@joepetrowski joepetrowski joepetrowski approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jsidorenko @mustermeiszer @gilescope @joepetrowski