make add to context infallible

galacticcouncil · Jan 6, 2025 · ff165cc · ff165cc
1 parent 81053ac
commit ff165cc
Show file tree

Hide file tree

Showing 9 changed files with 26 additions and 27 deletions.
diff --git a/pallets/dca/src/lib.rs b/pallets/dca/src/lib.rs
@@ -694,7 +694,7 @@ impl<T: Config> Pallet<T> {
 		schedule_id: ScheduleId,
 		schedule: &Schedule<T::AccountId, T::AssetId, BlockNumberFor<T>>,
 	) -> Result<AmountInAndOut<Balance>, DispatchError> {
-		pallet_support::Pallet::<T>::add_to_context(|id| ExecutionType::DCA(schedule_id, id))?;
+		pallet_support::Pallet::<T>::add_to_context(|id| ExecutionType::DCA(schedule_id, id));
 
 		let origin: OriginFor<T> = Origin::<T>::Signed(schedule.owner.clone()).into();
 		let trade_result = match &schedule.order {

diff --git a/pallets/omnipool/src/lib.rs b/pallets/omnipool/src/lib.rs
@@ -1094,7 +1094,7 @@ pub mod pallet {
 				protocol_fee_amount: state_changes.fee.protocol_fee,
 			});
 
-			pallet_support::Pallet::<T>::add_to_context(ExecutionType::Omnipool)?;
+			pallet_support::Pallet::<T>::add_to_context(ExecutionType::Omnipool);
 
 			//Swapped event for AssetA to HubAsset
 			pallet_support::Pallet::<T>::deposit_trade_event(
@@ -1335,7 +1335,7 @@ pub mod pallet {
 				protocol_fee_amount: state_changes.fee.protocol_fee,
 			});
 
-			pallet_support::Pallet::<T>::add_to_context(ExecutionType::Omnipool)?;
+			pallet_support::Pallet::<T>::add_to_context(ExecutionType::Omnipool);
 
 			//Swapped even from AssetA to HubAsset
 			pallet_support::Pallet::<T>::deposit_trade_event(

diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs
@@ -259,7 +259,7 @@ pub mod pallet {
 
 			let route_length = route.len();
 
-			let next_event_id = pallet_support::Pallet::<T>::add_to_context(ExecutionType::Router)?;
+			let next_event_id = pallet_support::Pallet::<T>::add_to_context(ExecutionType::Router);
 
 			for (trade_index, (trade_amount, trade)) in trade_amounts.iter().rev().zip(route).enumerate() {
 				Self::disable_ed_handling_for_insufficient_assets(route_length, trade_index, trade);
@@ -496,7 +496,7 @@ impl<T: Config> Pallet<T> {
 
 		let route_length = route.len();
 
-		let next_event_id = pallet_support::Pallet::<T>::add_to_context(ExecutionType::Router)?;
+		let next_event_id = pallet_support::Pallet::<T>::add_to_context(ExecutionType::Router);
 
 		for (trade_index, (trade_amount, trade)) in trade_amounts.iter().zip(route.clone()).enumerate() {
 			Self::disable_ed_handling_for_insufficient_assets(route_length, trade_index, trade);

diff --git a/pallets/support/src/lib.rs b/pallets/support/src/lib.rs
@@ -121,27 +121,27 @@ impl<T: Config> Pallet<T> {
 		});
 	}
 
-	pub fn add_to_context<F>(execution_type: F) -> Result<IncrementalIdType, DispatchError>
+	pub fn add_to_context<F>(execution_type: F) -> IncrementalIdType
 	where
 		F: FnOnce(u32) -> ExecutionType,
 	{
-		let next_id = IncrementalId::<T>::try_mutate(|current_id| -> Result<IncrementalIdType, DispatchError> {
+		let next_id = IncrementalId::<T>::mutate(|current_id| -> IncrementalIdType {
 			let inc_id = *current_id;
 			*current_id = current_id.overflowing_add(1).0;
-			Ok(inc_id)
-		})?;
 
-		ExecutionContext::<T>::try_mutate(|stack| -> DispatchResult {
+			inc_id
+		});
+
+		ExecutionContext::<T>::mutate(|stack| {
 			//We make it fire and forget, and it should fail only in test and when if wrongly used
 			debug_assert_ne!(stack.len(), MAX_STACK_SIZE as usize, "Stack should not be full");
 			if let Err(err) = stack.try_push(execution_type(next_id)) {
 				log::warn!(target: LOG_TARGET, "The max stack size of execution stack has been reached: {:?}", err);
 			}
 
-			Ok(())
-		})?;
+		});
 
-		Ok(next_id)
+		next_id
 	}
 
 

diff --git a/pallets/support/src/tests/incremental_id.rs b/pallets/support/src/tests/incremental_id.rs
@@ -21,14 +21,14 @@ use crate::Event;
 #[test]
 fn stack_should_be_populated_when_pushed() {
 	ExtBuilder::default().build().execute_with(|| {
-		assert_ok!(AmmSupport::add_to_context(ExecutionType::Router));
+		AmmSupport::add_to_context(ExecutionType::Router);
 		assert_eq!(AmmSupport::execution_context(), vec![ExecutionType::Router(0)]);
 		assert_eq!(
 			AmmSupport::execution_context().into_inner(),
 			vec![ExecutionType::Router(0)]
 		);
 
-		assert_ok!(AmmSupport::add_to_context(ExecutionType::Router));
+		AmmSupport::add_to_context(ExecutionType::Router);
 		assert_eq!(
 			AmmSupport::execution_context(),
 			vec![ExecutionType::Router(0), ExecutionType::Router(1)]
@@ -38,7 +38,7 @@ fn stack_should_be_populated_when_pushed() {
 			vec![ExecutionType::Router(0), ExecutionType::Router(1)]
 		);
 
-		assert_ok!(AmmSupport::add_to_context(ExecutionType::Omnipool));
+		AmmSupport::add_to_context(ExecutionType::Omnipool);
 		assert_eq!(
 			AmmSupport::execution_context(),
 			vec![
@@ -61,9 +61,9 @@ fn stack_should_be_populated_when_pushed() {
 #[test]
 fn stack_should_be_reduced_when_poped() {
 	ExtBuilder::default().build().execute_with(|| {
-		assert_ok!(AmmSupport::add_to_context(ExecutionType::Router));
-		assert_ok!(AmmSupport::add_to_context(ExecutionType::Router));
-		assert_ok!(AmmSupport::add_to_context(ExecutionType::Omnipool));
+		AmmSupport::add_to_context(ExecutionType::Router);
+		AmmSupport::add_to_context(ExecutionType::Router);
+		AmmSupport::add_to_context(ExecutionType::Omnipool);
 
 		AmmSupport::remove_from_context(ExecutionType::Omnipool);
 		assert_eq!(
@@ -75,7 +75,7 @@ fn stack_should_be_reduced_when_poped() {
 			vec![ExecutionType::Router(0), ExecutionType::Router(1)]
 		);
 
-		assert_ok!(AmmSupport::add_to_context(ExecutionType::Omnipool));
+		AmmSupport::add_to_context(ExecutionType::Omnipool);
 		assert_eq!(
 			AmmSupport::execution_context(),
 			vec![
@@ -126,7 +126,7 @@ fn event_should_be_deposited() {
 #[test]
 fn nothing_is_removed_when_type_not_matched_with_last_stack_item() {
 	ExtBuilder::default().build().execute_with(|| {
-		assert_ok!(AmmSupport::add_to_context(ExecutionType::Router));
+		AmmSupport::add_to_context(ExecutionType::Router);
 
 		AmmSupport::remove_from_context(ExecutionType::Batch);
 
@@ -141,7 +141,7 @@ fn nothing_is_removed_when_type_not_matched_with_last_stack_item() {
 #[test]
 fn entry_is_removed_when_type_matched_with_last_stack_item() {
 	ExtBuilder::default().build().execute_with(|| {
-		assert_ok!(AmmSupport::add_to_context(ExecutionType::Router));
+		AmmSupport::add_to_context(ExecutionType::Router);
 
 		AmmSupport::remove_from_context(ExecutionType::Router);
 

diff --git a/pallets/support/src/tests/mock.rs b/pallets/support/src/tests/mock.rs
@@ -19,7 +19,7 @@ use crate as pallet_support;
 pub use crate::*;
 
 pub use frame_support::{
-	assert_ok, construct_runtime,
+	construct_runtime,
 	sp_runtime::{
 		traits::{BlakeTwo256, IdentityLookup},
 		BuildStorage,

diff --git a/runtime/adapters/src/xcm_exchange.rs b/runtime/adapters/src/xcm_exchange.rs
@@ -64,7 +64,7 @@ where
 		};
 		let use_onchain_route = vec![];
 
-		pallet_support::Pallet::<Runtime>::add_to_context(ExecutionType::XcmExchange).map_err(|_| give.clone())?;
+		pallet_support::Pallet::<Runtime>::add_to_context(ExecutionType::XcmExchange);
 
 		let trade_result = if maximal {
 			// sell

diff --git a/runtime/hydradx/src/system.rs b/runtime/hydradx/src/system.rs
@@ -387,7 +387,7 @@ pub struct ManageExecutionTypeForUnifiedEvent;
 
 impl BatchHook for ManageExecutionTypeForUnifiedEvent {
 	fn on_batch_start() -> DispatchResult {
-		AmmSupport::add_to_context(ExecutionType::Batch)?;
+		AmmSupport::add_to_context(ExecutionType::Batch);
 
 		Ok(())
 	}

diff --git a/runtime/hydradx/src/xcm.rs b/runtime/hydradx/src/xcm.rs
@@ -246,8 +246,7 @@ impl<Inner: ExecuteXcm<<XcmConfig as Config>::RuntimeCall>> ExecuteXcm<<XcmConfi
 		} else {
 			unique(&message)
 		};
-		pallet_support::Pallet::<Runtime>::add_to_context(|event_id| ExecutionType::Xcm(unique_id, event_id))
-			.map_err(|_| message.clone())?;
+		pallet_support::Pallet::<Runtime>::add_to_context(|event_id| ExecutionType::Xcm(unique_id, event_id));
 
 		let prepare_result = Inner::prepare(message);