Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

[Uniques V2] Atomic NFTs swap #12285

Merged
merged 34 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
3086c76
Atomic NFTs swap
jsidorenko Sep 16, 2022
12b756f
Fmt
jsidorenko Sep 16, 2022
8083eca
Merge branch 'js/uniques-v2-main-branch' into js/uniques-v2-swaps
jsidorenko Sep 19, 2022
5575d3f
Merge branch 'js/uniques-v2-main-branch' into js/uniques-v2-swaps
jsidorenko Sep 19, 2022
5564a52
Merge branch 'js/uniques-v2-main-branch' into js/uniques-v2-swaps
jsidorenko Sep 19, 2022
fc1024a
Fix benchmark
jsidorenko Sep 20, 2022
a2955be
Rename swap -> atomic_swap
jsidorenko Sep 20, 2022
bdc0538
Update target balance
jsidorenko Sep 20, 2022
a28cbcc
Rollback
jsidorenko Sep 20, 2022
504b7d5
Fix
jsidorenko Sep 20, 2022
1ccf996
".git/.scripts/bench-bot.sh" pallet dev pallet_nfts
Sep 20, 2022
ca2d1ec
Make desired item optional
jsidorenko Sep 29, 2022
78f4570
Apply suggestions
jsidorenko Sep 30, 2022
7c15814
Update frame/nfts/src/features/atomic_swap.rs
jsidorenko Sep 30, 2022
6232aa4
Rename fields
jsidorenko Sep 30, 2022
9acfb5c
Optimisation
jsidorenko Sep 30, 2022
8d6cbda
Add a comment
jsidorenko Sep 30, 2022
ee88eaa
Merge branch 'js/uniques-v2-main-branch' into js/uniques-v2-swaps
jsidorenko Sep 30, 2022
2ce1ea3
deadline -> maybe_deadline
jsidorenko Sep 30, 2022
99d1eb5
Add docs
jsidorenko Sep 30, 2022
b404705
Change comments
jsidorenko Sep 30, 2022
cccebd3
Add price direction field
jsidorenko Sep 30, 2022
2202f0d
".git/.scripts/bench-bot.sh" pallet dev pallet_nfts
Sep 30, 2022
2688fe5
Wrap price and direction
jsidorenko Oct 4, 2022
2dd02af
Fix benchmarks
jsidorenko Oct 4, 2022
e565989
Use ensure! instead of if {}
jsidorenko Oct 4, 2022
82841fc
Make duration param mandatory and limit it to MaxDeadlineDuration
jsidorenko Oct 4, 2022
7fbf339
Make the code safer
jsidorenko Oct 4, 2022
e956594
Fix clippy
jsidorenko Oct 4, 2022
b52a615
Chore
jsidorenko Oct 4, 2022
3b41ee1
Remove unused vars
jsidorenko Oct 5, 2022
239bf17
try
jsidorenko Oct 5, 2022
d086e81
try 2
jsidorenko Oct 5, 2022
c2afaa4
try 3
jsidorenko Oct 5, 2022
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
2 changes: 2 additions & 0 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1473,6 +1473,7 @@ parameter_types! {
pub const ValueLimit: u32 = 256;
pub const ApprovalsLimit: u32 = 20;
pub const MaxTips: u32 = 10;
pub const MaxDeadlineDuration: BlockNumber = 12 * 30 * DAYS;
}

impl pallet_uniques::Config for Runtime {
Expand Down Expand Up @@ -1512,6 +1513,7 @@ impl pallet_nfts::Config for Runtime {
type ValueLimit = ValueLimit;
type ApprovalsLimit = ApprovalsLimit;
type MaxTips = MaxTips;
type MaxDeadlineDuration = MaxDeadlineDuration;
type WeightInfo = pallet_nfts::weights::SubstrateWeight<Runtime>;
#[cfg(feature = "runtime-benchmarks")]
type Helper = ();
Expand Down
84 changes: 83 additions & 1 deletion frame/nfts/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use frame_support::{
BoundedVec,
};
use frame_system::RawOrigin as SystemOrigin;
use sp_runtime::traits::Bounded;
use sp_runtime::traits::{Bounded, One};
use sp_std::prelude::*;

use crate::Pallet as Nfts;
Expand Down Expand Up @@ -476,5 +476,87 @@ benchmarks_instance_pallet! {
}
}

create_swap {
let (collection, caller, _) = create_collection::<T, I>();
let (item1, ..) = mint_item::<T, I>(0);
let (item2, ..) = mint_item::<T, I>(1);
let price = ItemPrice::<T, I>::from(100u32);
let price_direction = PriceDirection::Receive;
let price_with_direction = PriceWithDirection { amount: price, direction: price_direction };
let duration = T::MaxDeadlineDuration::get();
frame_system::Pallet::<T>::set_block_number(One::one());
}: _(SystemOrigin::Signed(caller.clone()), collection, item1, collection, Some(item2), Some(price_with_direction.clone()), duration)
verify {
let current_block = frame_system::Pallet::<T>::block_number();
assert_last_event::<T, I>(Event::SwapCreated {
offered_collection: collection,
offered_item: item1,
desired_collection: collection,
desired_item: Some(item2),
price: Some(price_with_direction),
deadline: current_block.saturating_add(duration),
}.into());
}

cancel_swap {
let (collection, caller, _) = create_collection::<T, I>();
let (item1, ..) = mint_item::<T, I>(0);
let (item2, ..) = mint_item::<T, I>(1);
let price = ItemPrice::<T, I>::from(100u32);
let origin = SystemOrigin::Signed(caller.clone()).into();
let duration = T::MaxDeadlineDuration::get();
let price_direction = PriceDirection::Receive;
let price_with_direction = PriceWithDirection { amount: price, direction: price_direction };
frame_system::Pallet::<T>::set_block_number(One::one());
Nfts::<T, I>::create_swap(origin, collection, item1, collection, Some(item2), Some(price_with_direction.clone()), duration)?;
}: _(SystemOrigin::Signed(caller.clone()), collection, item1)
verify {
assert_last_event::<T, I>(Event::SwapCancelled {
offered_collection: collection,
offered_item: item1,
desired_collection: collection,
desired_item: Some(item2),
price: Some(price_with_direction),
deadline: duration.saturating_add(One::one()),
}.into());
}

claim_swap {
let (collection, caller, _) = create_collection::<T, I>();
let (item1, ..) = mint_item::<T, I>(0);
let (item2, ..) = mint_item::<T, I>(1);
let price = ItemPrice::<T, I>::from(0u32);
let price_direction = PriceDirection::Receive;
let price_with_direction = PriceWithDirection { amount: price, direction: price_direction };
let duration = T::MaxDeadlineDuration::get();
let target: T::AccountId = account("target", 0, SEED);
let target_lookup = T::Lookup::unlookup(target.clone());
let origin = SystemOrigin::Signed(caller.clone());
frame_system::Pallet::<T>::set_block_number(One::one());
Nfts::<T, I>::transfer(origin.clone().into(), collection, item2, target_lookup)?;
Nfts::<T, I>::create_swap(
origin.clone().into(),
collection,
item1,
collection,
Some(item2),
Some(price_with_direction.clone()),
duration,
)?;
}: _(SystemOrigin::Signed(target.clone()), collection, item2, collection, item1, Some(price_with_direction.clone()))
verify {
let current_block = frame_system::Pallet::<T>::block_number();
assert_last_event::<T, I>(Event::SwapClaimed {
sent_collection: collection,
sent_item: item2,
sent_item_owner: target,
received_collection: collection,
received_item: item1,
received_item_owner: caller,
price: Some(price_with_direction),
deadline: duration.saturating_add(One::one()),
}.into());
}

impl_benchmark_test_suite!(Nfts, crate::mock::new_test_ext(), crate::mock::Test);
}
175 changes: 175 additions & 0 deletions frame/nfts/src/features/atomic_swap.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
// This file is part of Substrate.

// Copyright (C) 2022 Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: Apache-2.0

// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use crate::*;
use frame_support::{
pallet_prelude::*,
traits::{Currency, ExistenceRequirement::KeepAlive},
};

impl<T: Config<I>, I: 'static> Pallet<T, I> {
pub fn do_create_swap(
gilescope marked this conversation as resolved.
Show resolved Hide resolved
caller: T::AccountId,
offered_collection_id: T::CollectionId,
offered_item_id: T::ItemId,
desired_collection_id: T::CollectionId,
maybe_desired_item_id: Option<T::ItemId>,
maybe_price: Option<PriceWithDirection<ItemPrice<T, I>>>,
duration: <T as SystemConfig>::BlockNumber,
) -> DispatchResult {
ensure!(duration <= T::MaxDeadlineDuration::get(), Error::<T, I>::WrongDuration);

let item = Item::<T, I>::get(&offered_collection_id, &offered_item_id)
.ok_or(Error::<T, I>::UnknownItem)?;
ensure!(item.owner == caller, Error::<T, I>::NoPermission);

match maybe_desired_item_id {
Some(desired_item_id) => ensure!(
Item::<T, I>::contains_key(&desired_collection_id, &desired_item_id),
Error::<T, I>::UnknownItem
),
None => ensure!(
Collection::<T, I>::contains_key(&desired_collection_id),
Error::<T, I>::UnknownCollection
),
};

let now = frame_system::Pallet::<T>::block_number();
let deadline = duration.saturating_add(now);

PendingSwapOf::<T, I>::insert(
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
&offered_collection_id,
&offered_item_id,
PendingSwap {
desired_collection: desired_collection_id,
desired_item: maybe_desired_item_id,
price: maybe_price.clone(),
deadline,
},
);

Self::deposit_event(Event::SwapCreated {
offered_collection: offered_collection_id,
offered_item: offered_item_id,
desired_collection: desired_collection_id,
desired_item: maybe_desired_item_id,
price: maybe_price,
deadline,
});

Ok(())
}

pub fn do_cancel_swap(
caller: T::AccountId,
offered_collection_id: T::CollectionId,
offered_item_id: T::ItemId,
) -> DispatchResult {
let swap = PendingSwapOf::<T, I>::get(&offered_collection_id, &offered_item_id)
.ok_or(Error::<T, I>::UnknownSwap)?;

let now = frame_system::Pallet::<T>::block_number();
if swap.deadline > now {
let item = Item::<T, I>::get(&offered_collection_id, &offered_item_id)
.ok_or(Error::<T, I>::UnknownItem)?;
ensure!(item.owner == caller, Error::<T, I>::NoPermission);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

if it's past the deadline and I'm not the owner is it right that I can cancel the swap (or is that not possible)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, right. Anyone could cancel an outdated swap and clear the storage

Copy link
Member

Choose a reason for hiding this comment

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

But there is no incentive for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

But there is no incentive for it.

Curious how this is usually tackled. I see two options

  1. Make it worth user's while
  2. Have an on_idle job clearing outdated swaps
  3. Both?

Copy link
Contributor

Choose a reason for hiding this comment

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

At the same time this could be done as a follow-up I guess, depending on the urgency.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ruseinov I saw deposit for the storage recored in Asset pallet. The amount stored within the same record.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, we either need to add deposits for everything data related, so this would incentivise users to clear the data.
But at the same time, the deposit would go back to the owner and not the method's caller. So, I don't see an incentive for the external user here to call this method.
I think this approach is mostly suitable for the on_idle job that would clear outdated data.


PendingSwapOf::<T, I>::remove(&offered_collection_id, &offered_item_id);

Self::deposit_event(Event::SwapCancelled {
offered_collection: offered_collection_id,
offered_item: offered_item_id,
desired_collection: swap.desired_collection,
desired_item: swap.desired_item,
price: swap.price,
deadline: swap.deadline,
});

Ok(())
}

pub fn do_claim_swap(
caller: T::AccountId,
send_collection_id: T::CollectionId,
send_item_id: T::ItemId,
receive_collection_id: T::CollectionId,
receive_item_id: T::ItemId,
witness_price: Option<PriceWithDirection<ItemPrice<T, I>>>,
) -> DispatchResult {
let send_item = Item::<T, I>::get(&send_collection_id, &send_item_id)
.ok_or(Error::<T, I>::UnknownItem)?;
let receive_item = Item::<T, I>::get(&receive_collection_id, &receive_item_id)
.ok_or(Error::<T, I>::UnknownItem)?;
let swap = PendingSwapOf::<T, I>::get(&receive_collection_id, &receive_item_id)
.ok_or(Error::<T, I>::UnknownSwap)?;

ensure!(send_item.owner == caller, Error::<T, I>::NoPermission);
ensure!(
swap.desired_collection == send_collection_id && swap.price == witness_price,
Error::<T, I>::UnknownSwap
);

if let Some(desired_item) = swap.desired_item {
ensure!(desired_item == send_item_id, Error::<T, I>::UnknownSwap);
}

let now = frame_system::Pallet::<T>::block_number();
ensure!(now <= swap.deadline, Error::<T, I>::DeadlineExpired);

if let Some(ref price) = swap.price {
match price.direction {
PriceDirection::Send => T::Currency::transfer(
&receive_item.owner,
&send_item.owner,
price.amount,
KeepAlive,
)?,
PriceDirection::Receive => T::Currency::transfer(
&send_item.owner,
&receive_item.owner,
price.amount,
KeepAlive,
)?,
};
}

// This also removes the swap.
Self::do_transfer(send_collection_id, send_item_id, receive_item.owner.clone(), |_, _| {
jsidorenko marked this conversation as resolved.
Show resolved Hide resolved
Ok(())
})?;
Self::do_transfer(
receive_collection_id,
receive_item_id,
send_item.owner.clone(),
|_, _| Ok(()),
)?;
gilescope marked this conversation as resolved.
Show resolved Hide resolved

Self::deposit_event(Event::SwapClaimed {
sent_collection: send_collection_id,
sent_item: send_item_id,
sent_item_owner: send_item.owner,
received_collection: receive_collection_id,
received_item: receive_item_id,
received_item_owner: receive_item.owner,
price: swap.price,
deadline: swap.deadline,
});

Ok(())
}
}
1 change: 1 addition & 0 deletions frame/nfts/src/features/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,5 @@
// See the License for the specific language governing permissions and
// limitations under the License.

pub mod atomic_swap;
pub mod buy_sell;
4 changes: 4 additions & 0 deletions frame/nfts/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {

Item::<T, I>::insert(&collection, &item, &details);
ItemPriceOf::<T, I>::remove(&collection, &item);
PendingSwapOf::<T, I>::remove(&collection, &item);

Self::deposit_event(Event::Transferred {
collection,
Expand Down Expand Up @@ -129,6 +130,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
ItemMetadataOf::<T, I>::remove_prefix(&collection, None);
#[allow(deprecated)]
ItemPriceOf::<T, I>::remove_prefix(&collection, None);
#[allow(deprecated)]
PendingSwapOf::<T, I>::remove_prefix(&collection, None);
CollectionMetadataOf::<T, I>::remove(&collection);
#[allow(deprecated)]
Attribute::<T, I>::remove_prefix((&collection,), None);
Expand Down Expand Up @@ -219,6 +222,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Item::<T, I>::remove(&collection, &item);
Account::<T, I>::remove((&owner, &collection, &item));
ItemPriceOf::<T, I>::remove(&collection, &item);
PendingSwapOf::<T, I>::remove(&collection, &item);

Self::deposit_event(Event::Burned { collection, item, owner });
Ok(())
Expand Down
Loading