add terminate DCA logic with the option to not specifying blocknumber

integration-tests/src/dca.rs

@@ -3815,6 +3815,42 @@ mod with_onchain_route {
 	}
 }
 
+#[test]
+fn terminate_should_work_for_freshly_created_dca() {
+	TestNet::reset();
+	Hydra::execute_with(|| {
+		//Arrange
+		init_omnipool_with_oracle_for_block_10();
+
+		let block_id = 11;
+		set_relaychain_block_number(block_id);
+
+		let budget = 1000 * UNITS;
+		let schedule1 = schedule_fake_with_buy_order(PoolType::Omnipool, HDX, DAI, 100 * UNITS, budget);
+
+		assert_ok!(DCA::schedule(
+				RuntimeOrigin::signed(ALICE.into()),
+				schedule1.clone(),
+				None
+			));
+
+		let schedule_id = 0;
+		let schedule = DCA::schedules(schedule_id);
+		assert!(schedule.is_some());
+
+		//Act
+		assert_ok!(DCA::terminate(
+				RuntimeOrigin::signed(ALICE.into()),
+				schedule_id,
+				None
+			));
+
+		//Assert
+		let schedule = DCA::schedules(schedule_id);
+		assert!(schedule.is_none());
+	});
+}
+
 fn create_xyk_pool_with_amounts(asset_a: u32, amount_a: u128, asset_b: u32, amount_b: u128) {
 	assert_ok!(Currencies::update_balance(
 		hydradx_runtime::RuntimeOrigin::root(),

pallets/dca/src/lib.rs

@@ -425,6 +425,11 @@ pub mod pallet {
 	#[pallet::getter(fn retries_on_error)]
 	pub type RetriesOnError<T: Config> = StorageMap<_, Blake2_128Concat, ScheduleId, u8, ValueQuery>;
 
+	/// Keep tracking the blocknumber when the schedule is planned to be executed
+	#[pallet::storage]
+	#[pallet::getter(fn schedule_execution_block)]
+	pub type ScheduleExecutionBlock<T: Config> = StorageMap<_, Blake2_128Concat, ScheduleId, BlockNumberFor<T>, OptionQuery>;
+
 	/// Keep tracking of the schedule ids to be executed in the block
 	#[pallet::storage]
 	#[pallet::getter(fn schedule_ids_per_block)]
@@ -585,7 +590,9 @@ pub mod pallet {
 
 			Self::try_unreserve_all(schedule_id, &schedule);
 
-			let next_execution_block = next_execution_block.ok_or(Error::<T>::ScheduleNotFound)?;
+			let next_execution_block = next_execution_block
+				.or(Self::schedule_execution_block(schedule_id))
+				.ok_or(Error::<T>::ScheduleNotFound)?;
 
 			//Remove schedule id from next execution block
 			ScheduleIdsPerBlock::<T>::try_mutate_exists(
@@ -1052,6 +1059,8 @@ impl<T: Config> Pallet<T> {
 			Ok(())
 		})?;
 
+		ScheduleExecutionBlock::<T>::insert(schedule_id, next_free_block);
+
 		Self::deposit_event(Event::ExecutionPlanned {
 			id: schedule_id,
 			who: who.clone(),
@@ -1194,6 +1203,7 @@ impl<T: Config> Pallet<T> {
 		ScheduleOwnership::<T>::remove(owner, schedule_id);
 		RemainingAmounts::<T>::remove(schedule_id);
 		RetriesOnError::<T>::remove(schedule_id);
+		ScheduleExecutionBlock::<T>::remove(schedule_id);
 	}
 }
 

pallets/dca/src/tests/mod.rs

@@ -123,6 +123,7 @@ macro_rules! assert_that_schedule_has_been_removed_from_storages {
 		assert!(DCA::schedules($schedule_id).is_none());
 		assert!(DCA::owner_of($owner, $schedule_id).is_none());
 		assert!(DCA::remaining_amounts($schedule_id).is_none());
+		assert!(DCA::schedule_execution_block($schedule_id).is_none());
 		assert_eq!(DCA::retries_on_error($schedule_id), 0);
 	};
 }

pallets/dca/src/tests/terminate.rs

@@ -18,6 +18,7 @@ use crate::tests::*;
 use crate::{assert_scheduled_ids, assert_that_schedule_has_been_removed_from_storages};
 use crate::{Error, Event};
 use frame_support::{assert_noop, assert_ok};
+use frame_support::traits::Hooks;
 use orml_traits::NamedMultiReservableCurrency;
 use pretty_assertions::assert_eq;
 use sp_runtime::DispatchError::BadOrigin;
@@ -299,6 +300,91 @@ fn terminate_should_fail_when_with_nonexisting_schedule() {
 		});
 }
 
-pub fn set_block_number(n: u64) {
-	System::set_block_number(n);
+#[test]
+fn terminate_should_work_when_no_block_specified() {
+	ExtBuilder::default()
+		.with_endowed_accounts(vec![(ALICE, HDX, 10000 * ONE)])
+		.build()
+		.execute_with(|| {
+			//Arrange
+			set_block_number(500);
+			let schedule = ScheduleBuilder::new().with_period(300).build();
+			let schedule_id = 0;
+			assert_ok!(DCA::schedule(RuntimeOrigin::signed(ALICE), schedule, Option::Some(600)));
+			set_block_number(600);
+
+			//Act
+			assert_ok!(DCA::terminate(RuntimeOrigin::root(), schedule_id, None));
+
+			//Assert
+			assert_that_schedule_has_been_removed_from_storages!(ALICE, schedule_id);
+
+			expect_events(vec![Event::Terminated {
+				id: 0,
+				who: ALICE,
+				error: Error::<Test>::ManuallyTerminated.into(),
+			}
+				.into()]);
+		});
+}
+
+#[test]
+fn terminate_should_work_when_no_block_specified_and_schedule_eeceuted_multiple_times() {
+	ExtBuilder::default()
+		.with_endowed_accounts(vec![(ALICE, HDX, 10000 * ONE)])
+		.build()
+		.execute_with(|| {
+			//Arrange
+			set_block_number(500);
+			let schedule = ScheduleBuilder::new().with_period(300).build();
+			let schedule_id = 0;
+			assert_ok!(DCA::schedule(RuntimeOrigin::signed(ALICE), schedule, Option::Some(600)));
+			set_block_number(600);
+			set_block_number(900);
+
+			//Act
+			assert_ok!(DCA::terminate(RuntimeOrigin::root(), schedule_id, None));
+
+			//Assert
+			assert_that_schedule_has_been_removed_from_storages!(ALICE, schedule_id);
+
+			expect_events(vec![Event::Terminated {
+				id: 0,
+				who: ALICE,
+				error: Error::<Test>::ManuallyTerminated.into(),
+			}
+				.into()]);
+		});
+}
+
+#[test]
+fn terminate_should_work_with_no_blocknumber_when_just_scheduled() {
+	ExtBuilder::default()
+		.with_endowed_accounts(vec![(ALICE, HDX, 10000 * ONE)])
+		.build()
+		.execute_with(|| {
+			//Arrange
+			set_block_number(500);
+			let schedule = ScheduleBuilder::new().with_period(300).build();
+			let schedule_id = 0;
+			assert_ok!(DCA::schedule(RuntimeOrigin::signed(ALICE), schedule, Option::Some(600)));
+
+			//Act
+			assert_ok!(DCA::terminate(RuntimeOrigin::root(), schedule_id, None));
+
+			//Assert
+			assert_that_schedule_has_been_removed_from_storages!(ALICE, schedule_id);
+
+			expect_events(vec![Event::Terminated {
+				id: 0,
+				who: ALICE,
+				error: Error::<Test>::ManuallyTerminated.into(),
+			}
+				.into()]);
+		});
 }
+
+pub fn set_block_number(to: u64) {
+	System::set_block_number(to);
+	DCA::on_initialize(to);
+}