[Uniques V2] Atomic NFTs swap

bin/node/runtime/src/lib.rs

@@ -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 {
@@ -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 = ();

frame/nfts/src/benchmarking.rs

@@ -483,16 +483,17 @@ benchmarks_instance_pallet! {
 		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::BlockNumber::max_value();
-	}: _(SystemOrigin::Signed(caller.clone()), collection, item1, collection, Some(item2), Some(price_with_direction.clone()), Some(duration))
+		let duration = T::MaxDeadlineDuration::get();
+	}: _(SystemOrigin::Signed(caller.clone()), collection, item1, collection, Some(item2), Some(price_with_direction.clone()), duration)
 	verify {
+		let expect_deadline = duration + 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: Some(duration),
+			deadline: expect_deadline,
 		}.into());
 	}
 
@@ -502,19 +503,20 @@ benchmarks_instance_pallet! {
 		let (item2, ..) = mint_item::<T, I>(1);
 		let price = ItemPrice::<T, I>::from(100u32);
 		let origin = SystemOrigin::Signed(caller.clone()).into();
-		let duration = T::BlockNumber::max_value();
+		let duration = T::MaxDeadlineDuration::get();
 		let price_direction = PriceDirection::Receive;
 		let price_with_direction = PriceWithDirection { amount: price, direction: price_direction };
-		Nfts::<T, I>::create_swap(origin, collection, item1, collection, Some(item2), Some(price_with_direction.clone()), Some(duration))?;
+		Nfts::<T, I>::create_swap(origin, collection, item1, collection, Some(item2), Some(price_with_direction.clone()), duration)?;
 	}: _(SystemOrigin::Signed(caller.clone()), collection, item1)
 	verify {
+		let expect_deadline = duration + frame_system::Pallet::<T>::block_number();
 		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: Some(duration),
+			deadline: expect_deadline,
 		}.into());
 	}
 
@@ -525,7 +527,7 @@ benchmarks_instance_pallet! {
 		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::BlockNumber::max_value();
+		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());
@@ -537,10 +539,11 @@ benchmarks_instance_pallet! {
 			collection,
 			Some(item2),
 			Some(price_with_direction.clone()),
-			Some(duration),
+			duration,
 		)?;
 	}: _(SystemOrigin::Signed(target.clone()), collection, item2, collection, item1, Some(price_with_direction.clone()))
 	verify {
+		let expect_deadline = duration + frame_system::Pallet::<T>::block_number();
 		assert_last_event::<T, I>(Event::SwapClaimed {
 			sent_collection: collection,
 			sent_item: item2,
@@ -549,7 +552,7 @@ benchmarks_instance_pallet! {
 			received_item: item1,
 			received_item_owner: caller,
 			price: Some(price_with_direction),
-			deadline: Some(duration),
+			deadline: expect_deadline,
 		}.into());
 	}
 

frame/nfts/src/features/atomic_swap.rs

@@ -29,8 +29,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		desired_collection_id: T::CollectionId,
 		maybe_desired_item_id: Option<T::ItemId>,
 		maybe_price: Option<PriceWithDirection<ItemPrice<T, I>>>,
-		maybe_duration: Option<<T as SystemConfig>::BlockNumber>,
+		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);
@@ -47,7 +49,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		};
 
 		let now = frame_system::Pallet::<T>::block_number();
-		let maybe_deadline = maybe_duration.map(|d| d.saturating_add(now));
+		let deadline = duration.saturating_add(now);
 
 		PendingSwapOf::<T, I>::insert(
 			&offered_collection_id,
@@ -56,7 +58,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				desired_collection: desired_collection_id,
 				desired_item: maybe_desired_item_id,
 				price: maybe_price.clone(),
-				deadline: maybe_deadline,
+				deadline: deadline.clone(),
 			},
 		);
 
@@ -66,7 +68,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			desired_collection: desired_collection_id,
 			desired_item: maybe_desired_item_id,
 			price: maybe_price,
-			deadline: maybe_deadline,
+			deadline,
 		});
 
 		Ok(())
@@ -80,14 +82,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		let swap = PendingSwapOf::<T, I>::get(&offered_collection_id, &offered_item_id)
 			.ok_or(Error::<T, I>::UnknownSwap)?;
 
-		let is_past_deadline = if let Some(deadline) = swap.deadline {
-			let now = frame_system::Pallet::<T>::block_number();
-			now > deadline
-		} else {
-			false
-		};
-
-		if !is_past_deadline {
+		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);
@@ -132,10 +128,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			ensure!(desired_item == send_item_id, Error::<T, I>::UnknownSwap);
 		}
 
-		if let Some(deadline) = swap.deadline {
-			let now = frame_system::Pallet::<T>::block_number();
-			ensure!(now <= deadline, Error::<T, I>::DeadlineExpired);
-		}
+		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 {

frame/nfts/src/lib.rs

@@ -182,6 +182,10 @@ pub mod pallet {
 		#[pallet::constant]
 		type MaxTips: Get<u32>;
 
+		/// The max duration in blocks for deadlines.
+		#[pallet::constant]
+		type MaxDeadlineDuration: Get<<Self as SystemConfig>::BlockNumber>;
+
 		#[cfg(feature = "runtime-benchmarks")]
 		/// A set of helper functions for benchmarking.
 		type Helper: BenchmarkHelper<Self::CollectionId, Self::ItemId>;
@@ -457,7 +461,7 @@ pub mod pallet {
 			desired_collection: T::CollectionId,
 			desired_item: Option<T::ItemId>,
 			price: Option<PriceWithDirection<ItemPrice<T, I>>>,
-			deadline: Option<<T as SystemConfig>::BlockNumber>,
+			deadline: <T as SystemConfig>::BlockNumber,
 		},
 		/// The swap was cancelled.
 		SwapCancelled {
@@ -466,7 +470,7 @@ pub mod pallet {
 			desired_collection: T::CollectionId,
 			desired_item: Option<T::ItemId>,
 			price: Option<PriceWithDirection<ItemPrice<T, I>>>,
-			deadline: Option<<T as SystemConfig>::BlockNumber>,
+			deadline: <T as SystemConfig>::BlockNumber,
 		},
 		/// The swap has been claimed.
 		SwapClaimed {
@@ -477,7 +481,7 @@ pub mod pallet {
 			received_item: T::ItemId,
 			received_item_owner: T::AccountId,
 			price: Option<PriceWithDirection<ItemPrice<T, I>>>,
-			deadline: Option<<T as SystemConfig>::BlockNumber>,
+			deadline: <T as SystemConfig>::BlockNumber,
 		},
 	}
 
@@ -527,6 +531,8 @@ pub mod pallet {
 		ReachedApprovalLimit,
 		/// The deadline has already expired.
 		DeadlineExpired,
+		/// The duration provided should be less or equal to MaxDeadlineDuration.
+		WrongDuration,
 	}
 
 	impl<T: Config<I>, I: 'static> Pallet<T, I> {
@@ -1713,7 +1719,7 @@ pub mod pallet {
 			desired_collection: T::CollectionId,
 			maybe_desired_item: Option<T::ItemId>,
 			maybe_price: Option<PriceWithDirection<ItemPrice<T, I>>>,
-			maybe_duration: Option<<T as SystemConfig>::BlockNumber>,
+			duration: <T as SystemConfig>::BlockNumber,
 		) -> DispatchResult {
 			let origin = ensure_signed(origin)?;
 			Self::do_create_swap(
@@ -1723,7 +1729,7 @@ pub mod pallet {
 				desired_collection,
 				maybe_desired_item,
 				maybe_price,
-				maybe_duration,
+				duration,
 			)
 		}
 

frame/nfts/src/mock.rs

@@ -102,6 +102,7 @@ impl Config for Test {
 	type ValueLimit = ConstU32<50>;
 	type ApprovalsLimit = ConstU32<10>;
 	type MaxTips = ConstU32<10>;
+	type MaxDeadlineDuration = ConstU64<10000>;
 	type WeightInfo = ();
 	#[cfg(feature = "runtime-benchmarks")]
 	type Helper = ();

frame/nfts/src/tests.rs

@@ -18,7 +18,11 @@
 //! Tests for Nfts pallet.
 
 use crate::{mock::*, Event, *};
-use frame_support::{assert_noop, assert_ok, dispatch::Dispatchable, traits::Currency};
+use frame_support::{
+	assert_noop, assert_ok,
+	dispatch::Dispatchable,
+	traits::{Currency, Get},
+};
 use pallet_balances::Error as BalancesError;
 use sp_std::prelude::*;
 
@@ -1134,7 +1138,7 @@ fn create_cancel_swap_should_work() {
 				collection_id,
 				Some(item_2 + 1),
 				Some(price_with_direction.clone()),
-				Some(duration),
+				duration,
 			),
 			Error::<Test>::UnknownItem
 		);
@@ -1146,34 +1150,48 @@ fn create_cancel_swap_should_work() {
 				collection_id + 1,
 				None,
 				Some(price_with_direction.clone()),
-				Some(duration),
+				duration,
 			),
 			Error::<Test>::UnknownCollection
 		);
 
+		let max_duration: u64 = <Test as Config>::MaxDeadlineDuration::get();
+		assert_noop!(
+			Nfts::create_swap(
+				RuntimeOrigin::signed(user_id),
+				collection_id,
+				item_1,
+				collection_id,
+				Some(item_2),
+				Some(price_with_direction.clone()),
+				max_duration.saturating_add(1),
+			),
+			Error::<Test>::WrongDuration
+		);
+
 		assert_ok!(Nfts::create_swap(
 			RuntimeOrigin::signed(user_id),
 			collection_id,
 			item_1,
 			collection_id,
 			Some(item_2),
 			Some(price_with_direction.clone()),
-			Some(duration),
+			duration,
 		));
 
 		let swap = PendingSwapOf::<Test>::get(collection_id, item_1).unwrap();
 		assert_eq!(swap.desired_collection, collection_id);
 		assert_eq!(swap.desired_item, Some(item_2));
 		assert_eq!(swap.price, Some(price_with_direction.clone()));
-		assert_eq!(swap.deadline, Some(expect_deadline));
+		assert_eq!(swap.deadline, expect_deadline);
 
 		assert!(events().contains(&Event::<Test>::SwapCreated {
 			offered_collection: collection_id,
 			offered_item: item_1,
 			desired_collection: collection_id,
 			desired_item: Some(item_2),
 			price: Some(price_with_direction.clone()),
-			deadline: Some(expect_deadline),
+			deadline: expect_deadline,
 		}));
 
 		// validate we can cancel the swap
@@ -1184,7 +1202,7 @@ fn create_cancel_swap_should_work() {
 			desired_collection: collection_id,
 			desired_item: Some(item_2),
 			price: Some(price_with_direction.clone()),
-			deadline: Some(expect_deadline),
+			deadline: expect_deadline,
 		}));
 		assert!(!PendingSwapOf::<Test>::contains_key(collection_id, item_1));
 
@@ -1196,7 +1214,7 @@ fn create_cancel_swap_should_work() {
 			collection_id,
 			Some(item_2),
 			Some(price_with_direction.clone()),
-			Some(duration),
+			duration,
 		));
 		assert_noop!(
 			Nfts::cancel_swap(RuntimeOrigin::signed(user_id + 1), collection_id, item_1),
@@ -1213,7 +1231,7 @@ fn create_cancel_swap_should_work() {
 			collection_id,
 			None,
 			Some(price_with_direction),
-			Some(duration),
+			duration,
 		));
 
 		let swap = PendingSwapOf::<Test>::get(collection_id, item_1).unwrap();
@@ -1259,7 +1277,7 @@ fn claim_swap_should_work() {
 			collection_id,
 			Some(item_2),
 			Some(price_with_direction.clone()),
-			Some(duration),
+			duration,
 		));
 
 		// validate the deadline
@@ -1365,7 +1383,7 @@ fn claim_swap_should_work() {
 			received_item: item_1,
 			received_item_owner: user_1,
 			price: Some(price_with_direction.clone()),
-			deadline: Some(deadline),
+			deadline,
 		}));
 
 		// validate the optional desired_item param and another price direction
@@ -1381,7 +1399,7 @@ fn claim_swap_should_work() {
 			collection_id,
 			None,
 			Some(price_with_direction.clone()),
-			Some(duration),
+			duration,
 		));
 		assert_ok!(Nfts::claim_swap(
 			RuntimeOrigin::signed(user_2),

frame/nfts/src/types.rs

@@ -156,7 +156,7 @@ pub struct PendingSwap<CollectionId, ItemId, ItemPriceWithDirection, Deadline> {
 	/// A price for the desired `item` with the direction.
 	pub(super) price: Option<ItemPriceWithDirection>,
 	/// An optional deadline for the swap.
-	pub(super) deadline: Option<Deadline>,
+	pub(super) deadline: Deadline,
 }
 
 #[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, TypeInfo, MaxEncodedLen)]