Skip to content

Commit

Permalink
Merge pull request #235 from Szegoo/improve-nesting
Browse files Browse the repository at this point in the history 
Improve nesting
  • Loading branch information
@ilionic
ilionic authored Oct 31, 2022
2 parents 6300270 + 234527c commit cd298ac
Show file tree
Hide file tree
Showing 15 changed files with 214 additions and 78 deletions.
53 changes: 30 additions & 23 deletions pallets/rmrk-core/src/functions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use sp_runtime::{
ArithmeticError,
};

use rmrk_traits::budget::Budget;
use sp_std::collections::btree_set::BTreeSet;

// Randomness to generate NFT virtual accounts
Expand Down Expand Up @@ -64,7 +65,8 @@ where
!Pallet::<T>::is_locked(collection_id, *nft_id),
pallet_uniques::Error::<T>::Locked
);
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, *nft_id)?;
let budget = budget::Value::new(T::NestingBudget::get());
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, *nft_id, &budget)?;
ensure!(root_owner == collection.issuer, Error::<T>::NoPermission);
}
Properties::<T>::insert((&collection_id, maybe_nft_id, &key), &value);
Expand Down Expand Up @@ -343,8 +345,6 @@ impl<T: Config> Nft<T::AccountId, StringLimitOf<T>, BoundedResourceInfoTypeOf<T>
where
T: pallet_uniques::Config<CollectionId = CollectionId, ItemId = NftId>,
{
type MaxRecursions = T::MaxRecursions;

fn nft_mint(
sender: T::AccountId,
owner: T::AccountId,
Expand Down Expand Up @@ -448,7 +448,8 @@ where
}

// Calculate the rootowner of the intended owner of the minted NFT
let (rootowner, _) = Self::lookup_root_owner(owner.0, owner.1)?;
let budget = budget::Value::new(T::NestingBudget::get());
let (rootowner, _) = Self::lookup_root_owner(owner.0, owner.1, &budget)?;

// NFT should be pending if minting either to an NFT owned by another account
let pending = rootowner != sender;
Expand Down Expand Up @@ -521,10 +522,8 @@ where
owner: T::AccountId,
collection_id: CollectionId,
nft_id: NftId,
max_recursions: u32,
budget: &dyn Budget,
) -> sp_std::result::Result<(CollectionId, NftId), DispatchError> {
ensure!(max_recursions > 0, Error::<T>::TooManyRecursions);

// Remove self from parent's Children storage
if let Some(nft) = Self::nfts(collection_id, nft_id) {
if let AccountIdOrCollectionNftTuple::CollectionAndNftTuple(parent_col, parent_nft) =
Expand All @@ -545,7 +544,8 @@ where
for ((child_collection_id, child_nft_id), _) in
Children::<T>::drain_prefix((collection_id, nft_id))
{
Self::nft_burn(owner.clone(), child_collection_id, child_nft_id, max_recursions - 1)?;
ensure!(budget.consume() != false, Error::<T>::TooManyRecursions);
Self::nft_burn(owner.clone(), child_collection_id, child_nft_id, budget)?;
}

// decrement nfts counter
Expand Down Expand Up @@ -574,7 +574,9 @@ where
// Check if parent returns None which indicates the NFT is not available
ensure!(parent.is_some(), Error::<T>::NoAvailableNftId); // <- is this error wrong?

let (root_owner, _root_nft) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
let budget = budget::Value::new(T::NestingBudget::get());
let (root_owner, _root_nft) =
Pallet::<T>::lookup_root_owner(collection_id, nft_id, &budget)?;
// Check ownership
ensure!(sender == root_owner, Error::<T>::NoPermission);
// Get NFT info
Expand Down Expand Up @@ -609,7 +611,9 @@ where
!Pallet::<T>::is_x_descendent_of_y(cid, nid, collection_id, nft_id),
Error::<T>::CannotSendToDescendentOrSelf
);
let (recipient_root_owner, _root_nft) = Pallet::<T>::lookup_root_owner(cid, nid)?;
let budget = budget::Value::new(T::NestingBudget::get());
let (recipient_root_owner, _root_nft) =
Pallet::<T>::lookup_root_owner(cid, nid, &budget)?;
if recipient_root_owner == root_owner {
approval_required = false;
}
Expand Down Expand Up @@ -704,10 +708,11 @@ where
sender: T::AccountId,
collection_id: CollectionId,
nft_id: NftId,
max_recursions: u32,
) -> Result<(T::AccountId, CollectionId, NftId), DispatchError> {
// Look up root owner in Uniques to ensure permissions
let (root_owner, _root_nft) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
let budget = budget::Value::new(T::NestingBudget::get());
let (root_owner, _root_nft) =
Pallet::<T>::lookup_root_owner(collection_id, nft_id, &budget)?;

let nft = Nfts::<T>::get(collection_id, nft_id);

Expand Down Expand Up @@ -735,7 +740,7 @@ where
let _rejecting_nft =
Nfts::<T>::get(collection_id, nft_id).ok_or(Error::<T>::NoAvailableNftId)?;

Self::nft_burn(sender.clone(), collection_id, nft_id, max_recursions)?;
Self::nft_burn(sender.clone(), collection_id, nft_id, &budget)?;

Ok((sender, collection_id, nft_id))
}
Expand Down Expand Up @@ -841,20 +846,22 @@ where
///
/// Output:
/// - `Result<(T::AcccountId, (CollectionId, NftId)), Error<T>>`
#[allow(clippy::type_complexity)]
pub fn lookup_root_owner(
collection_id: CollectionId,
nft_id: NftId,
) -> Result<(T::AccountId, (CollectionId, NftId)), Error<T>> {
budget: &dyn Budget,
) -> Result<(T::AccountId, (CollectionId, NftId)), DispatchError> {
// Check if parent returns None which indicates the NFT is not available
let parent = match pallet_uniques::Pallet::<T>::owner(collection_id, nft_id) {
None => return Err(Error::<T>::NoAvailableNftId),
Some(parent) => parent,
};

match Self::decode_nft_account_id::<T::AccountId>(parent.clone()) {
None => Ok((parent, (collection_id, nft_id))),
Some((cid, nid)) => Pallet::<T>::lookup_root_owner(cid, nid),
if let Some(parent) = pallet_uniques::Pallet::<T>::owner(collection_id, nft_id) {
match Self::decode_nft_account_id::<T::AccountId>(parent.clone()) {
None => Ok((parent, (collection_id, nft_id))),
Some((cid, nid)) => {
ensure!(budget.consume() != false, Error::<T>::TooManyRecursions);
Pallet::<T>::lookup_root_owner(cid, nid, budget)
},
}
} else {
Err(Error::<T>::NoAvailableNftId.into())
}
}

Expand Down
48 changes: 30 additions & 18 deletions pallets/rmrk-core/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ use sp_runtime::{traits::StaticLookup, DispatchError, Permill};
use sp_std::convert::TryInto;

use rmrk_traits::{
primitives::*, AccountIdOrCollectionNftTuple, BasicResource, Collection, CollectionInfo,
ComposableResource, Nft, NftChild, NftInfo, PhantomType, Priority, Property, PropertyInfo,
Resource, ResourceInfo, ResourceInfoMin, ResourceTypes, RoyaltyInfo, SlotResource,
budget, primitives::*, AccountIdOrCollectionNftTuple, BasicResource, Collection,
CollectionInfo, ComposableResource, Nft, NftChild, NftInfo, PhantomType, Priority, Property,
PropertyInfo, Resource, ResourceInfo, ResourceInfoMin, ResourceTypes, RoyaltyInfo,
SlotResource,
};
use sp_std::result::Result;

Expand Down Expand Up @@ -93,7 +94,6 @@ pub mod pallet {
/// Because this pallet emits events, it depends on the runtime's definition of an event.
type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>;
type ProtocolOrigin: EnsureOrigin<Self::Origin>;
type MaxRecursions: Get<u32>;

/// The maximum resource symbol length
#[pallet::constant]
Expand All @@ -107,6 +107,10 @@ pub mod pallet {
#[pallet::constant]
type MaxPriorities: Get<u32>;

/// The maximum nesting allowed in the pallet extrinsics.
#[pallet::constant]
type NestingBudget: Get<u32>;

type CollectionSymbolLimit: Get<u32>;

type MaxResourcesOnMint: Get<u32>;
Expand Down Expand Up @@ -349,6 +353,7 @@ pub mod pallet {
ResourceAlreadyExists,
NftAlreadyExists,
EmptyResource,
/// The recursion limit has been reached.
TooManyRecursions,
NftIsLocked,
CannotAcceptNonOwnedNft,
Expand Down Expand Up @@ -494,14 +499,14 @@ pub mod pallet {
origin: OriginFor<T>,
collection_id: CollectionId,
nft_id: NftId,
max_burns: u32,
) -> DispatchResult {
let sender = ensure_signed(origin)?;
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
let budget = budget::Value::new(T::NestingBudget::get());
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id, &budget)?;
// Check ownership
ensure!(sender == root_owner, Error::<T>::NoPermission);
let (_collection_id, _nft_id) =
Self::nft_burn(root_owner, collection_id, nft_id, max_burns)?;
Self::nft_burn(root_owner, collection_id, nft_id, &budget)?;

Ok(())
}
Expand Down Expand Up @@ -560,7 +565,9 @@ pub mod pallet {
) -> DispatchResult {
let sender = ensure_signed(origin)?;

let (root_owner, _root_nft) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
let budget = budget::Value::new(T::NestingBudget::get());
let (root_owner, _root_nft) =
Pallet::<T>::lookup_root_owner(collection_id, nft_id, &budget)?;
// Check ownership
ensure!(sender == root_owner, Error::<T>::NoPermission);

Expand Down Expand Up @@ -598,9 +605,7 @@ pub mod pallet {
) -> DispatchResult {
let sender = ensure_signed(origin)?;

let max_recursions = T::MaxRecursions::get();
let (sender, collection_id, nft_id) =
Self::nft_reject(sender, collection_id, nft_id, max_recursions)?;
let (sender, collection_id, nft_id) = Self::nft_reject(sender, collection_id, nft_id)?;

Self::deposit_event(Event::NFTRejected { sender, collection_id, nft_id });
Ok(())
Expand Down Expand Up @@ -692,7 +697,8 @@ pub mod pallet {
Self::collections(collection_id).ok_or(Error::<T>::CollectionUnknown)?;

ensure!(collection.issuer == sender, Error::<T>::NoPermission);
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
let budget = budget::Value::new(T::NestingBudget::get());
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id, &budget)?;
// Check NFT lock status
ensure!(
!Pallet::<T>::is_locked(collection_id, nft_id),
Expand Down Expand Up @@ -729,7 +735,8 @@ pub mod pallet {
Self::collections(collection_id).ok_or(Error::<T>::CollectionUnknown)?;

ensure!(collection.issuer == sender, Error::<T>::NoPermission);
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
let budget = budget::Value::new(T::NestingBudget::get());
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id, &budget)?;
// Check NFT lock status
ensure!(
!Pallet::<T>::is_locked(collection_id, nft_id),
Expand Down Expand Up @@ -765,7 +772,8 @@ pub mod pallet {
Self::collections(collection_id).ok_or(Error::<T>::CollectionUnknown)?;

ensure!(collection.issuer == sender, Error::<T>::NoPermission);
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
let budget = budget::Value::new(T::NestingBudget::get());
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id, &budget)?;
// Check NFT lock status
ensure!(
!Pallet::<T>::is_locked(collection_id, nft_id),
Expand Down Expand Up @@ -796,7 +804,8 @@ pub mod pallet {
resource_id: ResourceId,
) -> DispatchResult {
let sender = ensure_signed(origin)?;
let (owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
let budget = budget::Value::new(T::NestingBudget::get());
let (owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id, &budget)?;
ensure!(owner == sender, Error::<T>::NoPermission);
// Check NFT lock status
ensure!(!Self::is_locked(collection_id, nft_id), pallet_uniques::Error::<T>::Locked);
Expand All @@ -818,7 +827,8 @@ pub mod pallet {
let sender = ensure_signed(origin)?;
let collection =
Self::collections(collection_id).ok_or(Error::<T>::CollectionUnknown)?;
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
let budget = budget::Value::new(T::NestingBudget::get());
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id, &budget)?;
ensure!(collection.issuer == sender, Error::<T>::NoPermission);

// Pending resource if sender is not root owner
Expand All @@ -840,7 +850,8 @@ pub mod pallet {
) -> DispatchResult {
let sender = ensure_signed(origin)?;

let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
let budget = budget::Value::new(T::NestingBudget::get());
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id, &budget)?;
ensure!(root_owner == sender, Error::<T>::NoPermission);

Self::accept_removal(sender, collection_id, nft_id, resource_id)?;
Expand All @@ -858,7 +869,8 @@ pub mod pallet {
priorities: BoundedVec<ResourceId, T::MaxPriorities>,
) -> DispatchResult {
let sender = ensure_signed(origin)?;
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
let budget = budget::Value::new(T::NestingBudget::get());
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id, &budget)?;
ensure!(sender == root_owner, Error::<T>::NoPermission);
// Check NFT lock status
ensure!(!Self::is_locked(collection_id, nft_id), pallet_uniques::Error::<T>::Locked);
Expand Down
5 changes: 2 additions & 3 deletions pallets/rmrk-core/src/mock.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ frame_support::construct_runtime!(
parameter_types! {
pub ClassBondAmount: Balance = 100;
pub MaxMetadataLength: u32 = 256;
pub const MaxRecursions: u32 = 4;
pub const ResourceSymbolLimit: u32 = 10;
pub const PartsLimit: u32 = 50;
pub const MaxPriorities: u32 = 3;
pub const NestingBudget: u32 = 3;
pub const CollectionSymbolLimit: u32 = 100;
pub const MaxResourcesOnMint: u32 = 3;
}
Expand All @@ -57,12 +57,12 @@ impl pallet_rmrk_core::Config for Test {
// type Currency = Balances;
type Event = Event;
type ProtocolOrigin = EnsureRoot<AccountId>;
type MaxRecursions = MaxRecursions;
type ResourceSymbolLimit = ResourceSymbolLimit;
type PartsLimit = PartsLimit;
type MaxPriorities = MaxPriorities;
type CollectionSymbolLimit = CollectionSymbolLimit;
type MaxResourcesOnMint = MaxResourcesOnMint;
type NestingBudget = NestingBudget;
}

parameter_types! {
Expand Down Expand Up @@ -155,7 +155,6 @@ pub const COLLECTION_ID_0: <Test as pallet_uniques::Config>::CollectionId = 0;
// pub const COLLECTION_ID_1: <Test as pallet_uniques::Config>::CollectionId = 1;
pub const NFT_ID_0: <Test as pallet_uniques::Config>::ItemId = 0;
pub const NOT_EXISTING_CLASS_ID: <Test as pallet_uniques::Config>::CollectionId = 999;
pub const MAX_BURNS: u32 = 4;

pub struct ExtBuilder;
impl Default for ExtBuilder {
Expand Down
Loading

0 comments on commit cd298ac

Please sign in to comment.