Mutable properties #269
Mutable properties #269
Conversation
Yes, this would make sense, but I don't think it is bad the way it is now. I am interested to hear the opinions of others here. |
There was a problem hiding this comment.
LGTM, thanks!
Not against splitting set_property and set_property_mutability if it makes code cleaner. But it seems like might require storage migration, so for @HashWarlock to chime in.
Integration tests could be addressed separately.
|
What would migration for storage look like? This will have a big impact for our PhalaWorld & Stakepool V2 implementation, so we need to prepare for this. We should also take a look at how @h4x3rotab may have some input as well on how to best implement this for future compatibility. |
|
Currently Khala has a blockchain storage of around 300MB. Among them, 200MB are RMRK NFT properties. Not sure if we migrate the full properties, we need to suspend the blockchain to do multi-bock migrations. If so, we would like to avoid it as much as possible, because a multi-block migration can cause tremendous chaos in our live parachain. |
|
It is possible to separate property mutability into it's own new storage,
which we can then just use as ValueQuery where default is false, but I'm not sure how that affects performance and if such a compromise is desirable.
…On Wed, Feb 1, 2023, 04:04 h4x3rotab ***@***.***> wrote:
Currently Khala has a blockchain storage of around 300MB. Among them,
200MB are RMRK NFT properties. Not sure if we migrate the full properties,
we need to suspend the blockchain to do multi-bock migrations. If so, we
would like to avoid it as much as possible, because a multi-block migration
can cause tremendous chaos in our live parachain.
—
Reply to this email directly, view it on GitHub
<#269 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHGXNIWCZJWAJN2L5HQPHXTWVIDHVANCNFSM6AAAAAAUD7VAUE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
This PR implements the ability to allow the current owner of an NFT to mutate properties that were set as mutable by the collection issuer prior to being sent to the new owner. Closes #198
Please do let me know if this implementation should be done differently, I just wanted to give a head start to this feature and am flexible to refactoring this PR entirely.
Regarding the arguments to the set_property call including the mutable field, this is handled by ignoring the value of that field if the user is not the collection issuer (which means they can't modify the mutability of the property), but I believe it would be a lot cleaner if we broke it down into 2 distinct calls: set_property and set_property_mutability, the former would be reverted to how it's arguments were previously, and the latter would be responsible for changing the value of the mutable field by the collection issuer (if they're also the current owner of the NFT).