Merge pull request #984 from galacticcouncil/feat/terminate-dca

feat: terminate DCA without knowing blocknumber

integration-tests/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "runtime-integration-tests"
-version = "1.28.0"
+version = "1.29.0"
 description = "Integration tests"
 authors = ["GalacticCouncil"]
 edition = "2021"

integration-tests/src/dca.rs

@@ -4026,6 +4026,38 @@ 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/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = 'pallet-dca'
-version = "1.7.0"
+version = "1.8.0"
 description = 'A pallet to manage DCA scheduling'
 authors = ['GalacticCouncil']
 edition = '2021'

pallets/dca/src/lib.rs

@@ -428,6 +428,12 @@ 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)]
@@ -588,7 +594,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(
@@ -1060,6 +1068,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(),
@@ -1202,6 +1212,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

@@ -17,6 +17,7 @@ use crate::tests::mock::*;
 use crate::tests::*;
 use crate::{assert_scheduled_ids, assert_that_schedule_has_been_removed_from_storages};
 use crate::{Error, Event};
+use frame_support::traits::Hooks;
 use frame_support::{assert_noop, assert_ok};
 use orml_traits::NamedMultiReservableCurrency;
 use pretty_assertions::assert_eq;
@@ -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);
 }

runtime/hydradx/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "hydradx-runtime"
-version = "279.0.0"
+version = "280.0.0"
 authors = ["GalacticCouncil"]
 edition = "2021"
 license = "Apache 2.0"

runtime/hydradx/src/benchmarking/dca.rs

@@ -489,7 +489,7 @@ runtime_benchmarks! {
 		let execution_block = 100u32;
 		assert_ok!(DCA::schedule(RawOrigin::Signed(caller).into(), schedule1, Option::Some(execution_block)));
 
-	}: _(RawOrigin::Root, schedule_id, Some(105))
+	}: _(RawOrigin::Root, schedule_id, None)
 	verify {
 		assert!(<Schedules<Runtime>>::get::<ScheduleId>(schedule_id).is_none());
 	}

runtime/hydradx/src/lib.rs

@@ -113,7 +113,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("hydradx"),
 	impl_name: create_runtime_str!("hydradx"),
 	authoring_version: 1,
-	spec_version: 279,
+	spec_version: 280,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 1,