Implement set_attribute & set_class_attribute nonfungibles trait functions for Uniques pallet
#9206
Conversation
…ibles trait functions
iorveth
changed the title
Uniques palletset_attribute & set_class_attribute nonfungibles trait functions for Uniques pallet
|
Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions. |
stale
bot
added
the
A5-stale
bkchr
removed
the
A5-stale
frame/uniques/src/functions.rs
Outdated
| } | ||
| class_details.total_deposit.saturating_accrue(deposit); | ||
| if deposit > old_deposit { | ||
| T::Currency::reserve(&class_details.owner, deposit - old_deposit)?; |
There was a problem hiding this comment.
| T::Currency::reserve(&class_details.owner, deposit - old_deposit)?; | |
| T::Currency::reserve(&class_details.owner, deposit.saturating_sub(old_deposit))?; |
There was a problem hiding this comment.
Thank you for the suggestion, done here 6fabf25
There was a problem hiding this comment.
This is NOT needed, and actually probably some additional overhead.
Note the check just 1 line above: deposit > old_deposit
This subtraction can never panic
please undo
frame/uniques/src/functions.rs
Outdated
| if deposit > old_deposit { | ||
| T::Currency::reserve(&class_details.owner, deposit - old_deposit)?; | ||
| } else if deposit < old_deposit { | ||
| T::Currency::unreserve(&class_details.owner, old_deposit - deposit); |
There was a problem hiding this comment.
| T::Currency::unreserve(&class_details.owner, old_deposit - deposit); | |
| T::Currency::unreserve(&class_details.owner, old_deposit.saturating_sub(deposit)); |
There was a problem hiding this comment.
Thank you for the suggestion, done here 6fabf25
frame/uniques/src/functions.rs
Outdated
| let mut deposit = Zero::zero(); | ||
| if !class_details.free_holding && maybe_check_owner.is_some() { | ||
| deposit = T::DepositPerByte::get() | ||
| .saturating_mul(((key.len() + value.len()) as u32).into()) |
There was a problem hiding this comment.
| .saturating_mul(((key.len() + value.len()) as u32).into()) | |
| .saturating_mul((key.len().saturating_add(value.len()) as u32).into()) |
There was a problem hiding this comment.
Thank you for the suggestion, done here 6fabf25
There was a problem hiding this comment.
this one i think is okay, but would want you to look closer before actually making this change
and then a test
|
This looks okay, but I suggest we do not increase the expectations of the Remember that we should not assume all (or even most) NFT systems will require some on-chain metadata. |
|
Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions. |
stale
bot
added
the
A5-stale
Implements missing
set_attribute,set_class_attributenonfungiblesMutatetrait functions foruniquespalletRelates to #9208