pallet-atomic-swap: generialized swap action (#6421)

* pallet-atomic-swap: generialized swap action

* Bump spec_version

* Fix weight calculation

* Remove unnecessary type aliases

substrate/bin/node/runtime/src/lib.rs

@@ -97,7 +97,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// and set impl_version to 0. If only runtime
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
-	spec_version: 253,
+	spec_version: 254,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 1,

substrate/frame/atomic-swap/src/lib.rs

@@ -42,10 +42,10 @@
 
 mod tests;
 
-use sp_std::prelude::*;
+use sp_std::{prelude::*, marker::PhantomData, ops::{Deref, DerefMut}};
 use sp_io::hashing::blake2_256;
 use frame_support::{
-	decl_module, decl_storage, decl_event, decl_error, ensure,
+	Parameter, decl_module, decl_storage, decl_event, decl_error, ensure,
 	traits::{Get, Currency, ReservableCurrency, BalanceStatus},
 	weights::Weight,
 	dispatch::DispatchResult,
@@ -55,37 +55,98 @@ use codec::{Encode, Decode};
 use sp_runtime::RuntimeDebug;
 
 /// Pending atomic swap operation.
-#[derive(Clone, RuntimeDebug, Eq, PartialEq, Encode, Decode)]
-pub struct PendingSwap<AccountId, Balance, BlockNumber> {
+#[derive(Clone, Eq, PartialEq, RuntimeDebug, Encode, Decode)]
+pub struct PendingSwap<T: Trait> {
 	/// Source of the swap.
-	pub source: AccountId,
-	/// Balance value of the swap.
-	pub balance: Balance,
+	pub source: T::AccountId,
+	/// Action of this swap.
+	pub action: T::SwapAction,
 	/// End block of the lock.
-	pub end_block: BlockNumber,
+	pub end_block: T::BlockNumber,
 }
 
-/// Balance type from the pallet's point of view.
-pub type BalanceFor<T> = <<T as Trait>::Currency as Currency<<T as frame_system::Trait>::AccountId>>::Balance;
+/// Hashed proof type.
+pub type HashedProof = [u8; 32];
 
-/// AccountId type from the pallet's point of view.
-pub type AccountIdFor<T> = <T as frame_system::Trait>::AccountId;
+/// Definition of a pending atomic swap action. It contains the following three phrases:
+///
+/// - **Reserve**: reserve the resources needed for a swap. This is to make sure that **Claim**
+/// succeeds with best efforts.
+/// - **Claim**: claim any resources reserved in the first phrase.
+/// - **Cancel**: cancel any resources reserved in the first phrase.
+pub trait SwapAction<T: Trait> {
+	/// Reserve the resources needed for the swap, from the given `source`. The reservation is
+	/// allowed to fail. If that is the case, the the full swap creation operation is cancelled.
+	fn reserve(&self, source: &T::AccountId) -> DispatchResult;
+	/// Claim the reserved resources, with `source` and `target`. Returns whether the claim
+	/// succeeds.
+	fn claim(&self, source: &T::AccountId, target: &T::AccountId) -> bool;
+	/// Weight for executing the operation.
+	fn weight(&self) -> Weight;
+	/// Cancel the resources reserved in `source`.
+	fn cancel(&self, source: &T::AccountId);
+}
 
-/// BlockNumber type from the pallet's point of view.
-pub type BlockNumberFor<T> = <T as frame_system::Trait>::BlockNumber;
+/// A swap action that only allows transferring balances.
+#[derive(Clone, RuntimeDebug, Eq, PartialEq, Encode, Decode)]
+pub struct BalanceSwapAction<T: Trait, C: ReservableCurrency<T::AccountId>> {
+	value: <C as Currency<<T as frame_system::Trait>::AccountId>>::Balance,
+	_marker: PhantomData<C>,
+}
 
-/// PendingSwap type from the pallet's point of view.
-pub type PendingSwapFor<T> = PendingSwap<AccountIdFor<T>, BalanceFor<T>, BlockNumberFor<T>>;
+impl<T: Trait, C> BalanceSwapAction<T, C> where
+	C: ReservableCurrency<T::AccountId>,
+{
+	/// Create a new swap action value of balance.
+	pub fn new(value: <C as Currency<<T as frame_system::Trait>::AccountId>>::Balance) -> Self {
+		Self { value, _marker: PhantomData }
+	}
+}
 
-/// Hashed proof type.
-pub type HashedProof = [u8; 32];
+impl<T: Trait, C> Deref for BalanceSwapAction<T, C> where
+	C: ReservableCurrency<T::AccountId>,
+{
+	type Target = <C as Currency<<T as frame_system::Trait>::AccountId>>::Balance;
+
+	fn deref(&self) -> &Self::Target {
+		&self.value
+	}
+}
+
+impl<T: Trait, C> DerefMut for BalanceSwapAction<T, C> where
+	C: ReservableCurrency<T::AccountId>,
+{
+	fn deref_mut(&mut self) -> &mut Self::Target {
+		&mut self.value
+	}
+}
+
+impl<T: Trait, C> SwapAction<T> for BalanceSwapAction<T, C> where
+	C: ReservableCurrency<T::AccountId>,
+{
+	fn reserve(&self, source: &T::AccountId) -> DispatchResult {
+		C::reserve(&source, self.value)
+	}
+
+	fn claim(&self, source: &T::AccountId, target: &T::AccountId) -> bool {
+		C::repatriate_reserved(source, target, self.value, BalanceStatus::Free).is_ok()
+	}
+
+	fn weight(&self) -> Weight {
+		T::DbWeight::get().reads_writes(1, 1)
+	}
+
+	fn cancel(&self, source: &T::AccountId) {
+		C::unreserve(source, self.value);
+	}
+}
 
 /// Atomic swap's pallet configuration trait.
 pub trait Trait: frame_system::Trait {
 	/// The overarching event type.
 	type Event: From<Event<Self>> + Into<<Self as frame_system::Trait>::Event>;
-	/// The currency mechanism.
-	type Currency: ReservableCurrency<Self::AccountId>;
+	/// Swap action.
+	type SwapAction: SwapAction<Self> + Parameter;
 	/// Limit of proof size.
 	///
 	/// Atomic swap is only atomic if once the proof is revealed, both parties can submit the proofs
@@ -103,7 +164,7 @@ decl_storage! {
 	trait Store for Module<T: Trait> as AtomicSwap {
 		pub PendingSwaps: double_map
 			hasher(twox_64_concat) T::AccountId, hasher(blake2_128_concat) HashedProof
-			=> Option<PendingSwapFor<T>>;
+			=> Option<PendingSwap<T>>;
 	}
 }
 
@@ -121,6 +182,8 @@ decl_error! {
 		AlreadyClaimed,
 		/// Swap does not exist.
 		NotExist,
+		/// Claim action mismatch.
+		ClaimActionMismatch,
 		/// Duration has not yet passed for the swap to be cancelled.
 		DurationNotPassed,
 	}
@@ -129,14 +192,13 @@ decl_error! {
 decl_event!(
 	/// Event of atomic swap pallet.
 	pub enum Event<T> where
-		Balance = BalanceFor<T>,
-		AccountId = AccountIdFor<T>,
-		PendingSwap = PendingSwapFor<T>,
+		AccountId = <T as system::Trait>::AccountId,
+		PendingSwap = PendingSwap<T>,
 	{
 		/// Swap created.
 		NewSwap(AccountId, HashedProof, PendingSwap),
 		/// Swap claimed. The last parameter indicates whether the execution succeeds.
-		SwapClaimed(AccountId, HashedProof, Balance, bool),
+		SwapClaimed(AccountId, HashedProof, bool),
 		/// Swap cancelled.
 		SwapCancelled(AccountId, HashedProof),
 	}
@@ -164,10 +226,10 @@ decl_module! {
 		#[weight = T::DbWeight::get().reads_writes(1, 1).saturating_add(40_000_000)]
 		fn create_swap(
 			origin,
-			target: AccountIdFor<T>,
+			target: T::AccountId,
 			hashed_proof: HashedProof,
-			balance: BalanceFor<T>,
-			duration: BlockNumberFor<T>,
+			action: T::SwapAction,
+			duration: T::BlockNumber,
 		) {
 			let source = ensure_signed(origin)?;
 			ensure!(
@@ -175,11 +237,11 @@ decl_module! {
 				Error::<T>::AlreadyExist
 			);
 
-			T::Currency::reserve(&source, balance)?;
+			action.reserve(&source)?;
 
 			let swap = PendingSwap {
 				source,
-				balance,
+				action,
 				end_block: frame_system::Module::<T>::block_number() + duration,
 			};
 			PendingSwaps::<T>::insert(target.clone(), hashed_proof.clone(), swap.clone());
@@ -194,13 +256,17 @@ decl_module! {
 		/// The dispatch origin for this call must be _Signed_.
 		///
 		/// - `proof`: Revealed proof of the claim.
-		#[weight = T::DbWeight::get().reads_writes(2, 2)
+		/// - `action`: Action defined in the swap, it must match the entry in blockchain. Otherwise
+		///   the operation fails. This is used for weight calculation.
+		#[weight = T::DbWeight::get().reads_writes(1, 1)
 		  .saturating_add(40_000_000)
 		  .saturating_add((proof.len() as Weight).saturating_mul(100))
+		  .saturating_add(action.weight())
 		]
 		fn claim_swap(
 			origin,
 			proof: Vec<u8>,
+			action: T::SwapAction,
 		) -> DispatchResult {
 			ensure!(
 				proof.len() <= T::ProofLimit::get() as usize,
@@ -212,18 +278,14 @@ decl_module! {
 
 			let swap = PendingSwaps::<T>::get(&target, hashed_proof)
 				.ok_or(Error::<T>::InvalidProof)?;
+			ensure!(swap.action == action, Error::<T>::ClaimActionMismatch);
 
-			let succeeded = T::Currency::repatriate_reserved(
-				&swap.source,
-				&target,
-				swap.balance,
-				BalanceStatus::Free,
-			).is_ok();
+			let succeeded = swap.action.claim(&swap.source, &target);
 
 			PendingSwaps::<T>::remove(target.clone(), hashed_proof.clone());
 
 			Self::deposit_event(
-				RawEvent::SwapClaimed(target, hashed_proof, swap.balance, succeeded)
+				RawEvent::SwapClaimed(target, hashed_proof, succeeded)
 			);
 
 			Ok(())
@@ -238,7 +300,7 @@ decl_module! {
 		#[weight = T::DbWeight::get().reads_writes(1, 1).saturating_add(40_000_000)]
 		fn cancel_swap(
 			origin,
-			target: AccountIdFor<T>,
+			target: T::AccountId,
 			hashed_proof: HashedProof,
 		) {
 			let source = ensure_signed(origin)?;
@@ -254,10 +316,7 @@ decl_module! {
 				Error::<T>::DurationNotPassed,
 			);
 
-			T::Currency::unreserve(
-				&swap.source,
-				swap.balance,
-			);
+			swap.action.cancel(&swap.source);
 			PendingSwaps::<T>::remove(&target, hashed_proof.clone());
 
 			Self::deposit_event(

substrate/frame/atomic-swap/src/tests.rs

@@ -21,7 +21,7 @@ impl_outer_origin! {
 // For testing the pallet, we construct most of a mock runtime. This means
 // first constructing a configuration type (`Test`) which `impl`s each of the
 // configuration traits of pallets we want to use.
-#[derive(Clone, Eq, PartialEq)]
+#[derive(Clone, Eq, Debug, PartialEq)]
 pub struct Test;
 parameter_types! {
 	pub const BlockHashCount: u64 = 250;
@@ -71,7 +71,7 @@ parameter_types! {
 }
 impl Trait for Test {
 	type Event = ();
-	type Currency = Balances;
+	type SwapAction = BalanceSwapAction<Test, Balances>;
 	type ProofLimit = ProofLimit;
 }
 type System = frame_system::Module<Test>;
@@ -109,7 +109,7 @@ fn two_party_successful_swap() {
 			Origin::signed(A),
 			B,
 			hashed_proof.clone(),
-			50,
+			BalanceSwapAction::new(50),
 			1000,
 		).unwrap();
 
@@ -123,7 +123,7 @@ fn two_party_successful_swap() {
 			Origin::signed(B),
 			A,
 			hashed_proof.clone(),
-			75,
+			BalanceSwapAction::new(75),
 			1000,
 		).unwrap();
 
@@ -136,6 +136,7 @@ fn two_party_successful_swap() {
 		AtomicSwap::claim_swap(
 			Origin::signed(A),
 			proof.to_vec(),
+			BalanceSwapAction::new(75),
 		).unwrap();
 
 		assert_eq!(Balances::free_balance(A), 100 + 75);
@@ -147,6 +148,7 @@ fn two_party_successful_swap() {
 		AtomicSwap::claim_swap(
 			Origin::signed(B),
 			proof.to_vec(),
+			BalanceSwapAction::new(50),
 		).unwrap();
 
 		assert_eq!(Balances::free_balance(A), 100 - 50);