Cancel RefundRequested transactions

pallets/payment/README.md

@@ -10,6 +10,7 @@ This pallet allows users to create secure reversible payments that keep funds lo
 - Resolver Account: A resolver account is assigned to every payment created, this account has the privilege to cancel/release a payment that has been disputed.
 - Remark: The pallet allows to create payments by optionally providing some extra(limited) amount of bytes, this is reffered to as Remark. This can be used by a marketplace to seperate/tag payments.
 - CancelBufferBlockLength: This is the time window where the recipient can dispute a cancellation request from the payment creator.
+- MaxTasksPerBlock: The max count of tasks that can be added to a block, this number should be small so that the entire block is not consumed by the on_initialize function.
 
 ## Interface
 
@@ -31,7 +32,6 @@ This pallet allows users to create secure reversible payments that keep funds lo
 - `resolve_release_payment` - Allows assigned judge to release a payment
 - `resolve_cancel_payment` - Allows assigned judge to cancel a payment
 - `request_refund` - Allows the creator of the payment to trigger cancel with a buffer time.
-- `claim_refund` - Allows the creator to claim payment refund after buffer time
 - `dispute_refund` - Allows the recipient to dispute the payment request of sender
 - `request_payment` - Create a payment that can be completed by the sender using the `accept_and_pay` extrinsic.
 - `accept_and_pay` - Allows the sender to fulfill a payment request created by a recipient
@@ -50,10 +50,12 @@ pub struct PaymentDetail<T: pallet::Config> {
 	/// type of asset used for payment
 	pub asset: AssetIdOf<T>,
 	/// amount of asset used for payment
+	#[codec(compact)]
 	pub amount: BalanceOf<T>,
 	/// incentive amount that is credited to creator for resolving
+	#[codec(compact)]
 	pub incentive_amount: BalanceOf<T>,
-	/// enum to track payment lifecycle [Created, NeedsReview]
+	/// enum to track payment lifecycle [Created, NeedsReview, RefundRequested, Requested]
 	pub state: PaymentState<T::BlockNumber>,
 	/// account that can settle any disputes created in the payment
 	pub resolver_account: T::AccountId,
@@ -71,7 +73,9 @@ pub enum PaymentState<BlockNumber> {
 	/// A judge needs to review and release manually
 	NeedsReview,
 	/// The user has requested refund and will be processed by `BlockNumber`
-	RefundRequested(BlockNumber),
+	RefundRequested { cancel_block: BlockNumber },
+	/// The recipient of this transaction has created a request
+	PaymentRequested,
 }
 ```
 

pallets/payment/src/benchmarking.rs

@@ -2,10 +2,8 @@ use super::*;
 
 use crate::{Pallet as Payment, Payment as PaymentStore};
 use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller};
-use frame_support::traits::{OnFinalize, OnInitialize};
 use frame_system::RawOrigin;
 use orml_traits::MultiCurrency;
-use sp_runtime::traits::One;
 use sp_std::vec;
 use virto_primitives::Asset;
 
@@ -26,16 +24,6 @@ fn assert_last_event<T: Config>(generic_event: <T as Config>::Event) {
 	assert_eq!(event, &system_event);
 }
 
-pub fn run_to_block<T: Config>(n: T::BlockNumber) {
-	while frame_system::Pallet::<T>::block_number() < n {
-		frame_system::Pallet::<T>::on_finalize(frame_system::Pallet::<T>::block_number());
-		frame_system::Pallet::<T>::set_block_number(
-			frame_system::Pallet::<T>::block_number() + One::one(),
-		);
-		frame_system::Pallet::<T>::on_initialize(frame_system::Pallet::<T>::block_number());
-	}
-}
-
 benchmarks! {
 	where_clause { where T::Asset: MultiCurrency<
 		<T as frame_system::Config>::AccountId,
@@ -112,19 +100,6 @@ benchmarks! {
 		assert_last_event::<T>(Event::<T>::PaymentCreatorRequestedRefund { from: caller, to: recipent, expiry: 601u32.into() }.into());
 	}
 
-	// creator of payment can claim a refund
-	claim_refund {
-		let caller = whitelisted_caller();
-		let _ = T::Asset::deposit(get_currency_id(), &caller, INITIAL_AMOUNT);
-		let recipent : T::AccountId = account("recipient", 0, SEED);
-		Payment::<T>::pay(RawOrigin::Signed(caller.clone()).into(), recipent.clone(), get_currency_id(), SOME_AMOUNT, None)?;
-		Payment::<T>::request_refund(RawOrigin::Signed(caller.clone()).into(), recipent.clone())?;
-		run_to_block::<T>(700u32.into());
-	}: _(RawOrigin::Signed(caller.clone()), recipent.clone())
-	verify {
-		assert_last_event::<T>(Event::<T>::PaymentCancelled { from: caller, to: recipent}.into());
-	}
-
 	// recipient of a payment can dispute a refund request
 	dispute_refund {
 		let caller = whitelisted_caller();

pallets/payment/src/lib.rs

@@ -17,7 +17,10 @@ pub mod weights;
 #[frame_support::pallet]
 pub mod pallet {
 	pub use crate::{
-		types::{DisputeResolver, FeeHandler, PaymentDetail, PaymentHandler, PaymentState},
+		types::{
+			DisputeResolver, FeeHandler, PaymentDetail, PaymentHandler, PaymentState,
+			ScheduledTask, Task,
+		},
 		weights::WeightInfo,
 	};
 	use frame_support::{
@@ -36,6 +39,7 @@ pub mod pallet {
 	pub type AssetIdOf<T> =
 		<<T as Config>::Asset as MultiCurrency<<T as frame_system::Config>::AccountId>>::CurrencyId;
 	pub type BoundedDataOf<T> = BoundedVec<u8, <T as Config>::MaxRemarkLength>;
+	pub type ScheduledTaskOf<T> = ScheduledTask<<T as frame_system::Config>::BlockNumber>;
 
 	#[pallet::config]
 	pub trait Config: frame_system::Config {
@@ -53,6 +57,9 @@ pub mod pallet {
 		/// Maximum permitted size of `Remark`
 		#[pallet::constant]
 		type MaxRemarkLength: Get<u32>;
+		/// Maximum scheduled tasks per block - this number should be small enough to ensure entire block is not consumed by on_init hook
+		#[pallet::constant]
+		type MaxTasksPerBlock: Get<u32>;
 		/// Buffer period - number of blocks to wait before user can claim canceled payment
 		#[pallet::constant]
 		type CancelBufferBlockLength: Get<Self::BlockNumber>;
@@ -65,7 +72,7 @@ pub mod pallet {
 	pub struct Pallet<T>(_);
 
 	#[pallet::storage]
-	#[pallet::getter(fn rates)]
+	#[pallet::getter(fn payment)]
 	/// Payments created by a user, this method of storageDoubleMap is chosen since there is no usecase for
 	/// listing payments by provider/currency. The payment will only be referenced by the creator in
 	/// any transaction of interest.
@@ -80,6 +87,18 @@ pub mod pallet {
 		PaymentDetail<T>,
 	>;
 
+	#[pallet::storage]
+	#[pallet::getter(fn tasks)]
+	/// Store the list of tasks to be executed in the on_idle function
+	pub(super) type ScheduledTasks<T: Config> = StorageDoubleMap<
+		_,
+		Blake2_128Concat,
+		T::AccountId, // payment creator
+		Blake2_128Concat,
+		T::AccountId, // payment recipient
+		ScheduledTaskOf<T>,
+	>;
+
 	#[pallet::event]
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
 	pub enum Event<T: Config> {
@@ -126,10 +145,47 @@ pub mod pallet {
 		RefundNotRequested,
 		/// Dispute period has not passed
 		DisputePeriodNotPassed,
+		/// The automatic cancelation queue cannot accept
+		RefundQueueFull,
 	}
 
 	#[pallet::hooks]
-	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {}
+	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
+		/// Hook that execute when there is leftover space in a block
+		/// This function will look for any pending scheduled tasks that can
+		/// be executed and will process them.
+		fn on_idle(now: T::BlockNumber, mut remaining_weight: Weight) -> Weight {
+			while remaining_weight > T::WeightInfo::cancel() {
+				let task = ScheduledTasks::<T>::iter().next();
+
+				match task {
+					Some((from, to, ScheduledTask { task, when })) => {
+						// early return if the expiry block is in future
+						if when > now {
+							return remaining_weight
+						}
+
+						// process the task
+						match task {
+							Task::Cancel => {
+								remaining_weight -= T::WeightInfo::cancel();
+								ScheduledTasks::<T>::remove(from.clone(), to.clone());
+								// process the cancel payment
+								let _ = <Self as PaymentHandler<T>>::settle_payment(
+									from.clone(),
+									to.clone(),
+									Percent::from_percent(0),
+								);
+								Self::deposit_event(Event::PaymentCancelled { from, to });
+							},
+						}
+					},
+					_ => return remaining_weight,
+				}
+			}
+			remaining_weight
+		}
+	}
 
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
@@ -144,7 +200,7 @@ pub mod pallet {
 			origin: OriginFor<T>,
 			recipient: T::AccountId,
 			asset: AssetIdOf<T>,
-			amount: BalanceOf<T>,
+			#[pallet::compact] amount: BalanceOf<T>,
 			remark: Option<BoundedDataOf<T>>,
 		) -> DispatchResultWithPostInfo {
 			let who = ensure_signed(origin)?;
@@ -176,6 +232,8 @@ pub mod pallet {
 			// ensure the payment is in Created state
 			if let Some(payment) = Payment::<T>::get(&from, &to) {
 				ensure!(payment.state == PaymentState::Created, Error::<T>::InvalidAction)
+			} else {
+				fail!(Error::<T>::InvalidPayment);
 			}
 
 			// release is a settle_payment with 100% recipient_share
@@ -290,15 +348,22 @@ pub mod pallet {
 
 					// set the payment to requested refund
 					let current_block = frame_system::Pallet::<T>::block_number();
-					let can_cancel_block = current_block
+					let cancel_block = current_block
 						.checked_add(&T::CancelBufferBlockLength::get())
 						.ok_or(Error::<T>::MathError)?;
-					payment.state = PaymentState::RefundRequested(can_cancel_block);
+
+					ScheduledTasks::<T>::insert(
+						who.clone(),
+						recipient.clone(),
+						ScheduledTask { task: Task::Cancel, when: cancel_block },
+					);
+
+					payment.state = PaymentState::RefundRequested { cancel_block };
 
 					Self::deposit_event(Event::PaymentCreatorRequestedRefund {
 						from: who,
 						to: recipient,
-						expiry: can_cancel_block,
+						expiry: cancel_block,
 					});
 
 					Ok(())
@@ -308,43 +373,6 @@ pub mod pallet {
 			Ok(().into())
 		}
 
-		/// Allow payment creator to claim the refund if the payment recipent has not disputed
-		/// After the payment creator has `request_refund` can then call this extrinsic to
-		/// cancel the payment and receive the reserved amount to the account if the dispute period
-		/// has passed.
-		#[transactional]
-		#[pallet::weight(T::WeightInfo::claim_refund())]
-		pub fn claim_refund(
-			origin: OriginFor<T>,
-			recipient: T::AccountId,
-		) -> DispatchResultWithPostInfo {
-			use PaymentState::*;
-			let who = ensure_signed(origin)?;
-
-			if let Some(payment) = Payment::<T>::get(&who, &recipient) {
-				match payment.state {
-					NeedsReview => fail!(Error::<T>::PaymentNeedsReview),
-					Created | PaymentRequested => fail!(Error::<T>::RefundNotRequested),
-					RefundRequested(cancel_block) => {
-						let current_block = frame_system::Pallet::<T>::block_number();
-						// ensure the dispute period has passed
-						ensure!(current_block > cancel_block, Error::<T>::DisputePeriodNotPassed);
-						// cancel the payment and refund the creator
-						<Self as PaymentHandler<T>>::settle_payment(
-							who.clone(),
-							recipient.clone(),
-							Percent::from_percent(0),
-						)?;
-						Self::deposit_event(Event::PaymentCancelled { from: who, to: recipient });
-					},
-				}
-			} else {
-				fail!(Error::<T>::InvalidPayment);
-			}
-
-			Ok(().into())
-		}
-
 		/// Allow payment recipient to dispute the refund request from the payment creator
 		/// This does not cancel the request, instead sends the payment to a NeedsReview state
 		/// The assigned resolver account can then change the state of the payment after review.
@@ -365,9 +393,17 @@ pub mod pallet {
 					let payment = maybe_payment.as_mut().ok_or(Error::<T>::InvalidPayment)?;
 					// ensure the payment is in Requested Refund state
 					match payment.state {
-						RefundRequested(_) => {
+						RefundRequested { cancel_block } => {
+							ensure!(
+								cancel_block > frame_system::Pallet::<T>::block_number(),
+								Error::<T>::InvalidAction
+							);
+
 							payment.state = PaymentState::NeedsReview;
 
+							// remove the payment from scheduled tasks
+							ScheduledTasks::<T>::remove(creator.clone(), who.clone());
+
 							Self::deposit_event(Event::PaymentRefundDisputed {
 								from: creator,
 								to: who,
@@ -392,7 +428,7 @@ pub mod pallet {
 			origin: OriginFor<T>,
 			from: T::AccountId,
 			asset: AssetIdOf<T>,
-			amount: BalanceOf<T>,
+			#[pallet::compact] amount: BalanceOf<T>,
 		) -> DispatchResultWithPostInfo {
 			let to = ensure_signed(origin)?;
 
@@ -531,7 +567,6 @@ pub mod pallet {
 		/// For cancelling a payment, recipient_share = 0
 		/// For releasing a payment, recipient_share = 100
 		/// In other cases, the custom recipient_share can be specified
-		#[require_transactional]
 		fn settle_payment(
 			from: T::AccountId,
 			to: T::AccountId,

pallets/payment/src/mock.rs

@@ -2,7 +2,8 @@ use crate as payment;
 use crate::PaymentDetail;
 use frame_support::{
 	parameter_types,
-	traits::{Contains, Everything, GenesisBuild, OnFinalize, OnInitialize},
+	traits::{Contains, Everything, GenesisBuild, Hooks, OnFinalize},
+	weights::DispatchClass,
 };
 use frame_system as system;
 use orml_traits::parameter_type_with_key;
@@ -21,6 +22,8 @@ pub type Balance = u128;
 pub type AccountId = u8;
 pub const PAYMENT_CREATOR: AccountId = 10;
 pub const PAYMENT_RECIPENT: AccountId = 11;
+pub const PAYMENT_CREATOR_TWO: AccountId = 30;
+pub const PAYMENT_RECIPENT_TWO: AccountId = 31;
 pub const CURRENCY_ID: Asset = Asset::Network(NetworkAsset::KSM);
 pub const RESOLVER_ACCOUNT: AccountId = 12;
 pub const FEE_RECIPIENT_ACCOUNT: AccountId = 20;
@@ -124,6 +127,7 @@ parameter_types! {
 	pub const IncentivePercentage: Percent = Percent::from_percent(10);
 	pub const MaxRemarkLength: u32 = 50;
 	pub const CancelBufferBlockLength: u64 = 600;
+	pub const MaxTasksPerBlock: u32 = 2;
 }
 
 impl payment::Config for Test {
@@ -134,16 +138,22 @@ impl payment::Config for Test {
 	type FeeHandler = MockFeeHandler;
 	type MaxRemarkLength = MaxRemarkLength;
 	type CancelBufferBlockLength = CancelBufferBlockLength;
+	type MaxTasksPerBlock = MaxTasksPerBlock;
 	type WeightInfo = ();
 }
 
 // Build genesis storage according to the mock runtime.
 pub fn new_test_ext() -> sp_io::TestExternalities {
 	let mut t = system::GenesisConfig::default().build_storage::<Test>().unwrap();
 
-	orml_tokens::GenesisConfig::<Test> { balances: vec![(PAYMENT_CREATOR, CURRENCY_ID, 100)] }
-		.assimilate_storage(&mut t)
-		.unwrap();
+	orml_tokens::GenesisConfig::<Test> {
+		balances: vec![
+			(PAYMENT_CREATOR, CURRENCY_ID, 100),
+			(PAYMENT_CREATOR_TWO, CURRENCY_ID, 100),
+		],
+	}
+	.assimilate_storage(&mut t)
+	.unwrap();
 
 	let mut ext: sp_io::TestExternalities = t.into();
 	// need to set block number to 1 to test events
@@ -153,8 +163,16 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
 
 pub fn run_to_block(n: u64) {
 	while System::block_number() < n {
-		System::on_finalize(System::block_number());
-		System::set_block_number(System::block_number() + 1);
-		System::on_initialize(System::block_number());
+		let block_number = System::block_number();
+
+		// ensure the on_idle is executed
+		<frame_system::Pallet<Test>>::register_extra_weight_unchecked(
+			Payment::on_idle(block_number, 1_000_000_000),
+			DispatchClass::Mandatory,
+		);
+
+		<frame_system::Pallet<Test> as OnFinalize<u64>>::on_finalize(block_number);
+
+		System::set_block_number(block_number + 1);
 	}
 }