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

Lazy ownership tree based on pallet-unique's owner #31

Merged
merged 13 commits into from
Jan 11, 2022
Merged
Show file tree
Hide file tree
Changes from 6 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
50 changes: 48 additions & 2 deletions pallets/rmrk-core/src/functions.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use super::*;
use codec::{Codec, Decode, Encode};
use sp_runtime::traits::TrailingZeroInput;
use rmrk_traits::AccountIdOrCollectionNftTuple::AccountId;

impl<T: Config> Collection<StringLimitOf<T>, T::AccountId> for Pallet<T> {
fn issuer(collection_id: CollectionId) -> Option<T::AccountId> {
Expand Down Expand Up @@ -58,7 +61,10 @@ impl<T: Config> Collection<StringLimitOf<T>, T::AccountId> for Pallet<T> {
}
}

impl<T: Config> Nft<T::AccountId, StringLimitOf<T>> for Pallet<T> {
impl<T: Config> Nft<T::AccountId, StringLimitOf<T>> for Pallet<T>
where
T: pallet_uniques::Config<ClassId = CollectionId, InstanceId = NftId>,
{
type MaxRecursions = T::MaxRecursions;

fn nft_mint(
Expand Down Expand Up @@ -184,7 +190,47 @@ impl<T: Config> Nft<T::AccountId, StringLimitOf<T>> for Pallet<T> {
}
}

impl<T: Config> Pallet<T> {
impl<T: Config> Pallet<T>
where
T: pallet_uniques::Config<ClassId = CollectionId, InstanceId = NftId>,
{

pub fn nft_to_account_id<AccountId: Codec>(collection_id: CollectionId, nft_id: NftId) -> AccountId {
HashWarlock marked this conversation as resolved.
Show resolved Hide resolved
(b"RmrkNft/", collection_id, nft_id)
HashWarlock marked this conversation as resolved.
Show resolved Hide resolved
.using_encoded(|b| AccountId::decode(&mut TrailingZeroInput::new(b)))
.expect("Decoding with trailing zero never fails; qed.")
}

pub fn decode_nft_account_id<AccountId: Codec>(account_id: T::AccountId) -> Option<(CollectionId, NftId)> {
let (prefix, tuple, suffix) = account_id
.using_encoded(|mut b| {
let slice = &mut b;
let r = <([u8; 8], (CollectionId, NftId))>::decode(slice);
r.map(|(prefix, tuple)| (prefix, tuple, slice.to_vec()))
})
.ok()?;
// Check prefix and suffix to avoid collision attack
if &prefix == b"RmrkNft/" && suffix.iter().all(|&x| x == 0) {
Some(tuple)
} else {
None
}
}

pub fn lookup_root_owner(collection_id: CollectionId, nft_id: NftId) -> Result<(T::AccountId, (CollectionId, NftId)), Error<T>> {
HashWarlock marked this conversation as resolved.
Show resolved Hide resolved
let parent =
pallet_uniques::Pallet::<T>::owner(collection_id, nft_id);
// Check if parent returns None which indicates the NFT is not available
Copy link
Contributor

Choose a reason for hiding this comment

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

What are the cases a NFT can have no parent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had thought of a couple cases, but the second case would be if we did the "rootless" children and tried to perform a lookup_root_owner

  1. NFT is already burned (Added a test for this one)
  2. A case if an ancestor NFT were burned leaving the

if parent.is_none() {
HashWarlock marked this conversation as resolved.
Show resolved Hide resolved
return Err(Error::<T>::NoAvailableNftId)
}
let owner = parent.as_ref().unwrap();
match Self::decode_nft_account_id::<T::AccountId>(owner.clone()) {
None => Ok((owner.clone(), (collection_id, nft_id))),
HashWarlock marked this conversation as resolved.
Show resolved Hide resolved
Some((cid, nid)) => Pallet::<T>::lookup_root_owner(cid, nid),
}
}

pub fn is_x_descendent_of_y(
HashWarlock marked this conversation as resolved.
Show resolved Hide resolved
child_collection_id: CollectionId,
child_nft_id: NftId,
Expand Down
45 changes: 43 additions & 2 deletions pallets/rmrk-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ pub mod pallet {
use super::*;
use frame_support::{dispatch::DispatchResult, pallet_prelude::*};
use frame_system::pallet_prelude::*;
use rmrk_traits::AccountIdOrCollectionNftTuple::AccountId;

/// Configure the pallet by specifying the parameters and types on which it depends.
#[pallet::config]
pub trait Config: frame_system::Config + pallet_uniques::Config {
/// 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>;
}
Expand Down Expand Up @@ -234,6 +234,7 @@ pub mod pallet {
ResourceAlreadyExists,
EmptyResource,
TooManyRecursions,
CircleDetected,
}

#[pallet::call]
Expand Down Expand Up @@ -338,6 +339,9 @@ pub mod pallet {
nft_id: NftId,
) -> DispatchResult {
let sender = ensure_signed(origin.clone())?;
let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
// Check ownership
ensure!(sender == root_owner, Error::<T>::NoPermission);
HashWarlock marked this conversation as resolved.
Show resolved Hide resolved
let max_recursions = T::MaxRecursions::get();
let (_collection_id, nft_id) = Self::nft_burn(collection_id, nft_id, max_recursions)?;

Expand Down Expand Up @@ -395,6 +399,21 @@ pub mod pallet {
}
.unwrap_or_default();

let (root_owner, root_nft) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
// Check ownership
ensure!(sender == root_owner, Error::<T>::NoPermission);
// Prepare transfer
let new_owner_account = match new_owner.clone() {
AccountIdOrCollectionNftTuple::AccountId(id) => id,
AccountIdOrCollectionNftTuple::CollectionAndNftTuple(cid, nid) => {
// Circle detection
let (_, new_owner_root_nft) = Pallet::<T>::lookup_root_owner(cid, nid)?;
ensure!(root_nft != new_owner_root_nft, Error::<T>::CircleDetected);
HashWarlock marked this conversation as resolved.
Show resolved Hide resolved
// Convert to virtual account
Pallet::<T>::nft_to_account_id::<T::AccountId>(cid, nid)
},
};
HashWarlock marked this conversation as resolved.
Show resolved Hide resolved

let max_recursions = T::MaxRecursions::get();
HashWarlock marked this conversation as resolved.
Show resolved Hide resolved
Self::nft_send(
sender.clone(),
Expand All @@ -403,10 +422,32 @@ pub mod pallet {
new_owner.clone(),
max_recursions,
)?;
// Check if root owner is a nft & call pallet_uniques::do_transfer w/ actual account owner
match Pallet::<T>::decode_nft_account_id::<T::AccountId>(new_owner_account.clone()) {
None => {
pallet_uniques::Pallet::<T>::do_transfer(
collection_id,
nft_id,
new_owner_account.clone(),
|class_details, details|
Ok(())
)?;
},
Some((cid, nid)) => {
let (dest, _) = Pallet::<T>::lookup_root_owner(cid, nid)?;
HashWarlock marked this conversation as resolved.
Show resolved Hide resolved
pallet_uniques::Pallet::<T>::do_transfer(
collection_id,
nft_id,
dest.clone(),
|class_details, details|
Ok(())
)?;
}
}
HashWarlock marked this conversation as resolved.
Show resolved Hide resolved

Self::deposit_event(Event::NFTSent {
sender,
recipient: new_owner,
recipient: new_owner.clone(),
collection_id,
nft_id,
});
Expand Down
13 changes: 13 additions & 0 deletions pallets/rmrk-core/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,17 @@ fn send_nft_to_minted_nft_works() {
AccountIdOrCollectionNftTuple::CollectionAndNftTuple(0, 0),
);

// Error if trying to assign send a nft to self nft
assert_noop!(
RMRKCore::send(
Origin::signed(BOB),
0,
0,
AccountIdOrCollectionNftTuple::CollectionAndNftTuple(0, 0)
),
Error::<Test>::CircleDetected
);

// Check that Bob now root-owns NFT (0, 1) [child] since he wasn't originally rootowner
assert_eq!(RMRKCore::nfts(0, 1).unwrap().rootowner, BOB);

Expand Down Expand Up @@ -398,7 +409,9 @@ fn burn_nft_works() {
Some(Permill::from_float(0.0)),
bvec![0u8; 20]
));
assert_noop!(RMRKCore::burn_nft(Origin::signed(BOB), COLLECTION_ID_0, NFT_ID_0), Error::<Test>::NoPermission);
assert_ok!(RMRKCore::burn_nft(Origin::signed(ALICE), COLLECTION_ID_0, NFT_ID_0));
assert_noop!(RMRKCore::burn_nft(Origin::signed(ALICE), COLLECTION_ID_0, NFT_ID_0), Error::<Test>::NoAvailableNftId);
assert_eq!(RMRKCore::nfts(COLLECTION_ID_0, NFT_ID_0).is_none(), true);
System::assert_last_event(MockEvent::RmrkCore(crate::Event::NFTBurned {
owner: ALICE,
Expand Down
2 changes: 1 addition & 1 deletion pallets/uniques/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use frame_support::{ensure, traits::Get};
use sp_runtime::{DispatchError, DispatchResult};

impl<T: Config<I>, I: 'static> Pallet<T, I> {
pub(crate) fn do_transfer(
HashWarlock marked this conversation as resolved.
Show resolved Hide resolved
pub fn do_transfer(
class: T::ClassId,
instance: T::InstanceId,
dest: T::AccountId,
Expand Down