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 traits for managing Memberships and implement for non-fungible #2513

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 11 additions & 13 deletions substrate/frame/nfts/src/impl_nonfungibles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,9 @@ impl<T: Config<I>, I: 'static> Destroy<<T as SystemConfig>::AccountId> for Palle
}
}

impl<T: Config<I>, I: 'static> Mutate<<T as SystemConfig>::AccountId, ItemConfig> for Pallet<T, I> {
impl<T: Config<I>, I: 'static> Mutate<<T as SystemConfig>::AccountId> for Pallet<T, I> {
type ItemConfig = ItemConfig;

fn mint_into(
collection: &Self::CollectionId,
item: &Self::ItemId,
Expand Down Expand Up @@ -257,7 +259,7 @@ impl<T: Config<I>, I: 'static> Mutate<<T as SystemConfig>::AccountId, ItemConfig
Self::do_burn(*collection, *item, |d| {
if let Some(check_owner) = maybe_check_owner {
if &d.owner != check_owner {
return Err(Error::<T, I>::NoPermission.into())
return Err(Error::<T, I>::NoPermission.into());
}
}
Ok(())
Expand Down Expand Up @@ -288,7 +290,7 @@ impl<T: Config<I>, I: 'static> Mutate<<T as SystemConfig>::AccountId, ItemConfig
) -> DispatchResult {
key.using_encoded(|k| {
value.using_encoded(|v| {
<Self as Mutate<T::AccountId, ItemConfig>>::set_attribute(collection, item, k, v)
<Self as Mutate<T::AccountId>>::set_attribute(collection, item, k, v)
})
})
}
Expand All @@ -315,9 +317,7 @@ impl<T: Config<I>, I: 'static> Mutate<<T as SystemConfig>::AccountId, ItemConfig
) -> DispatchResult {
key.using_encoded(|k| {
value.using_encoded(|v| {
<Self as Mutate<T::AccountId, ItemConfig>>::set_collection_attribute(
collection, k, v,
)
<Self as Mutate<T::AccountId>>::set_collection_attribute(collection, k, v)
})
})
}
Expand Down Expand Up @@ -368,9 +368,7 @@ impl<T: Config<I>, I: 'static> Mutate<<T as SystemConfig>::AccountId, ItemConfig
item: &Self::ItemId,
key: &K,
) -> DispatchResult {
key.using_encoded(|k| {
<Self as Mutate<T::AccountId, ItemConfig>>::clear_attribute(collection, item, k)
})
key.using_encoded(|k| <Self as Mutate<T::AccountId>>::clear_attribute(collection, item, k))
}

fn clear_collection_attribute(collection: &Self::CollectionId, key: &[u8]) -> DispatchResult {
Expand All @@ -388,7 +386,7 @@ impl<T: Config<I>, I: 'static> Mutate<<T as SystemConfig>::AccountId, ItemConfig
key: &K,
) -> DispatchResult {
key.using_encoded(|k| {
<Self as Mutate<T::AccountId, ItemConfig>>::clear_collection_attribute(collection, k)
<Self as Mutate<T::AccountId>>::clear_collection_attribute(collection, k)
})
}

Expand Down Expand Up @@ -422,10 +420,10 @@ impl<T: Config<I>, I: 'static> Transfer<T::AccountId> for Pallet<T, I> {
Self::has_system_attribute(&collection, &item, PalletAttributes::TransferDisabled)?;
// Can't lock the item twice
if transfer_disabled {
return Err(Error::<T, I>::ItemLocked.into())
return Err(Error::<T, I>::ItemLocked.into());
}

<Self as Mutate<T::AccountId, ItemConfig>>::set_attribute(
<Self as Mutate<T::AccountId>>::set_attribute(
collection,
item,
&PalletAttributes::<Self::CollectionId>::TransferDisabled.encode(),
Expand All @@ -434,7 +432,7 @@ impl<T: Config<I>, I: 'static> Transfer<T::AccountId> for Pallet<T, I> {
}

fn enable_transfer(collection: &Self::CollectionId, item: &Self::ItemId) -> DispatchResult {
<Self as Mutate<T::AccountId, ItemConfig>>::clear_attribute(
<Self as Mutate<T::AccountId>>::clear_attribute(
collection,
item,
&PalletAttributes::<Self::CollectionId>::TransferDisabled.encode(),
Expand Down
28 changes: 12 additions & 16 deletions substrate/frame/nfts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -998,7 +998,7 @@ fn set_collection_system_attributes_should_work() {
let attribute_key = [0u8];
let attribute_value = [0u8];

assert_ok!(<Nfts as Mutate<AccountIdOf<Test>, ItemConfig>>::set_collection_attribute(
assert_ok!(<Nfts as Mutate<AccountIdOf<Test>>>::set_collection_attribute(
&collection_id,
&attribute_key,
&attribute_value
Expand All @@ -1021,13 +1021,11 @@ fn set_collection_system_attributes_should_work() {
struct TypedAttributeValue(u32);
let typed_attribute_value = TypedAttributeValue(42);

assert_ok!(
<Nfts as Mutate<AccountIdOf<Test>, ItemConfig>>::set_typed_collection_attribute(
&collection_id,
&typed_attribute_key,
&typed_attribute_value
)
);
assert_ok!(<Nfts as Mutate<AccountIdOf<Test>>>::set_typed_collection_attribute(
&collection_id,
&typed_attribute_key,
&typed_attribute_value
));

assert_eq!(
<Nfts as Inspect<AccountIdOf<Test>>>::typed_system_attribute(
Expand Down Expand Up @@ -1384,7 +1382,7 @@ fn validate_deposit_required_setting() {
bvec![2],
bvec![0],
));
assert_ok!(<Nfts as Mutate<<Test as SystemConfig>::AccountId, ItemConfig>>::set_attribute(
assert_ok!(<Nfts as Mutate<<Test as SystemConfig>::AccountId>>::set_attribute(
&0,
&0,
&[3],
Expand All @@ -1403,13 +1401,11 @@ fn validate_deposit_required_setting() {
assert_eq!(Balances::reserved_balance(account(2)), 3);
assert_eq!(Balances::reserved_balance(account(3)), 3);

assert_ok!(
<Nfts as Mutate<<Test as SystemConfig>::AccountId, ItemConfig>>::clear_attribute(
&0,
&0,
&[3],
)
);
assert_ok!(<Nfts as Mutate<<Test as SystemConfig>::AccountId>>::clear_attribute(
&0,
&0,
&[3],
));
assert_eq!(
attributes(0),
vec![
Expand Down
5 changes: 3 additions & 2 deletions substrate/frame/support/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ mod members;
pub use members::{AllowAll, DenyAll, Filter};
pub use members::{
AsContains, ChangeMembers, Contains, ContainsLengthBound, ContainsPair, Equals, Everything,
EverythingBut, FromContainsPair, InitializeMembers, InsideBoth, IsInVec, Nothing,
RankedMembers, SortedMembers, TheseExcept,
EverythingBut, FromContainsPair, GenericRank, InitializeMembers, InsideBoth, IsInVec,
Membership, MembershipInspect, MembershipMutate, Nothing, RankedMembers, RankedMembership,
SortedMembers, TheseExcept,
};

mod validation;
Expand Down
141 changes: 141 additions & 0 deletions substrate/frame/support/src/traits/members.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@

//! Traits for dealing with the idea of membership.

use core::num::NonZeroU8;

use crate::Parameter;
use impl_trait_for_tuples::impl_for_tuples;
use sp_arithmetic::traits::AtLeast16BitUnsigned;
use sp_runtime::DispatchResult;
Expand Down Expand Up @@ -391,3 +394,141 @@ impl<T: Clone + Ord> ChangeMembers<T> for () {
fn set_members_sorted(_: &[T], _: &[T]) {}
fn set_prime(_: Option<T>) {}
}

/// Access data associated to a unique membership
pub trait MembershipInspect<M, AccountId, MembershipId>
where
M: Membership<Id = MembershipId>,
MembershipId: Parameter,
{
type MembershipsIter: Iterator<Item = MembershipId>;

/// Retrieve membership data that is expected to belong to member
fn get_membership(id: impl Into<MembershipId>, member: &AccountId) -> Option<M>;

/// Retrieve all memberships belonging to member
fn account_memberships(member: &AccountId) -> Self::MembershipsIter;

/// Check membership is owned by the given account
fn has_membership(id: impl Into<MembershipId>, member: &AccountId) -> bool {
Self::get_membership(id, member).is_some()
}
}

/// Change data related to a unique membership
pub trait MembershipMutate<M, AccountId, MembershipId>
where
M: Membership<Id = MembershipId>,
MembershipId: Parameter,
{
/// Update the membership possibly changing its owner
fn update(
id: impl Into<MembershipId>,
membership: M,
maybe_member: Option<AccountId>,
) -> DispatchResult;
}

/// A unique membership
pub trait Membership: codec::Decode + codec::Encode {
type Id;

fn id(&self) -> &Self::Id;

fn has_expired(&self) -> bool {
false
}
}

/// A membership with a rating system
pub trait RankedMembership<Rank = GenericRank>: Membership
where
Rank: Eq + Ord,
{
fn rank(&self) -> &Rank;
fn rank_mut(&mut self) -> &mut Rank;
}

/// A generic rank in the range 0 to 100
#[derive(
Clone,
Copy,
Debug,
Default,
Eq,
Ord,
PartialEq,
PartialOrd,
codec::Decode,
codec::Encode,
codec::MaxEncodedLen,
scale_info::TypeInfo,
)]
pub struct GenericRank(u8);
impl GenericRank {
pub const MIN: Self = GenericRank(0);
pub const MAX: Self = GenericRank(100);
pub const ADMIN: Self = Self::MAX;

pub fn set(&mut self, n: u8) {
*self = Self(n.min(Self::MAX.0))
}
pub fn promote_by(&mut self, n: NonZeroU8) {
*self = Self(self.0.saturating_add(n.get()).min(Self::MAX.0))
}
pub fn demote_by(&mut self, n: NonZeroU8) {
*self = Self(self.0.saturating_sub(n.get()).max(Self::MIN.0))
}
}
impl From<u8> for GenericRank {
fn from(value: u8) -> Self {
Self(value)
}
}

const MEMBERSHIP_INFO_ATTRIBUTE: [u8; 10] = *b"memberinfo";
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about adding MEMBERSHIP_INFO_ATTRIBUTE to the PalletAttributes enum?
Maybe we should extract that enum to a common space and rename to NftSystemAttributes..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extracting it somewhere in frame-support would be better :) we shouldn't create a dependency with the nfts pallet which is also why I made the ItemConfig an associated type.


impl<T, M, AccountId> MembershipInspect<M, AccountId, T::ItemId> for T
where
T: super::tokens::nonfungible_v2::Inspect<AccountId>
+ super::tokens::nonfungible_v2::InspectEnumerable<
AccountId,
OwnedIterator = crate::storage::KeyPrefixIterator<
<T as super::tokens::nonfungible_v2::Inspect<AccountId>>::ItemId,
>,
>,
M: Membership<Id = T::ItemId>,
AccountId: Eq,
{
type MembershipsIter = crate::storage::KeyPrefixIterator<T::ItemId>;

fn get_membership(id: impl Into<T::ItemId>, member: &AccountId) -> Option<M> {
let id = id.into();
T::owner(&id).and_then(|o| member.eq(&o).then_some(()))?;
T::typed_system_attribute(&id, &MEMBERSHIP_INFO_ATTRIBUTE)
}

fn account_memberships(member: &AccountId) -> Self::MembershipsIter {
T::owned(member)
}
}

impl<T, M, AccountId> MembershipMutate<M, AccountId, T::ItemId> for T
where
T: super::tokens::nonfungible_v2::Mutate<AccountId>
+ super::tokens::nonfungible_v2::Transfer<AccountId>,
M: Membership<Id = T::ItemId>,
AccountId: Eq,
{
fn update(
id: impl Into<T::ItemId>,
membership: M,
maybe_member: Option<AccountId>,
) -> DispatchResult {
let id = id.into();
if let Some(new_owner) = maybe_member {
T::transfer(&id, &new_owner)?;
}
T::set_typed_attribute(&id, &MEMBERSHIP_INFO_ATTRIBUTE, &membership)
}
}
40 changes: 15 additions & 25 deletions substrate/frame/support/src/traits/tokens/nonfungible_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,16 @@ pub trait InspectEnumerable<AccountId>: Inspect<AccountId> {

/// Trait for providing an interface for NFT-like items which may be minted, burned and/or have
/// attributes and metadata set on them.
pub trait Mutate<AccountId, ItemConfig>: Inspect<AccountId> {
pub trait Mutate<AccountId>: Inspect<AccountId> {
type ItemConfig;

/// Mint some `item` to be owned by `who`.
///
/// By default, this is not a supported operation.
fn mint_into(
_item: &Self::ItemId,
_who: &AccountId,
_config: &ItemConfig,
_config: &Self::ItemConfig,
_deposit_collection_owner: bool,
) -> DispatchResult {
Err(TokenError::Unsupported.into())
Expand Down Expand Up @@ -270,19 +272,21 @@ impl<
}

impl<
F: nonfungibles::Mutate<AccountId, ItemConfig>,
F: nonfungibles::Mutate<AccountId, ItemConfig = ItemConfig>,
A: Get<<F as nonfungibles::Inspect<AccountId>>::CollectionId>,
AccountId,
ItemConfig,
> Mutate<AccountId, ItemConfig> for ItemOf<F, A, AccountId>
> Mutate<AccountId> for ItemOf<F, A, AccountId>
{
type ItemConfig = ItemConfig;

fn mint_into(
item: &Self::ItemId,
who: &AccountId,
config: &ItemConfig,
config: &Self::ItemConfig,
deposit_collection_owner: bool,
) -> DispatchResult {
<F as nonfungibles::Mutate<AccountId, ItemConfig>>::mint_into(
<F as nonfungibles::Mutate<AccountId>>::mint_into(
&A::get(),
item,
who,
Expand All @@ -291,37 +295,23 @@ impl<
)
}
fn burn(item: &Self::ItemId, maybe_check_owner: Option<&AccountId>) -> DispatchResult {
<F as nonfungibles::Mutate<AccountId, ItemConfig>>::burn(&A::get(), item, maybe_check_owner)
<F as nonfungibles::Mutate<AccountId>>::burn(&A::get(), item, maybe_check_owner)
}
fn set_attribute(item: &Self::ItemId, key: &[u8], value: &[u8]) -> DispatchResult {
<F as nonfungibles::Mutate<AccountId, ItemConfig>>::set_attribute(
&A::get(),
item,
key,
value,
)
<F as nonfungibles::Mutate<AccountId>>::set_attribute(&A::get(), item, key, value)
}
fn set_typed_attribute<K: Encode, V: Encode>(
item: &Self::ItemId,
key: &K,
value: &V,
) -> DispatchResult {
<F as nonfungibles::Mutate<AccountId, ItemConfig>>::set_typed_attribute(
&A::get(),
item,
key,
value,
)
<F as nonfungibles::Mutate<AccountId>>::set_typed_attribute(&A::get(), item, key, value)
}
fn clear_attribute(item: &Self::ItemId, key: &[u8]) -> DispatchResult {
<F as nonfungibles::Mutate<AccountId, ItemConfig>>::clear_attribute(&A::get(), item, key)
<F as nonfungibles::Mutate<AccountId>>::clear_attribute(&A::get(), item, key)
}
fn clear_typed_attribute<K: Encode>(item: &Self::ItemId, key: &K) -> DispatchResult {
<F as nonfungibles::Mutate<AccountId, ItemConfig>>::clear_typed_attribute(
&A::get(),
item,
key,
)
<F as nonfungibles::Mutate<AccountId>>::clear_typed_attribute(&A::get(), item, key)
}
}

Expand Down
Loading
Loading