Merge pull request #781 from galacticcouncil/fix/add-checks-to-router

feat: spent and received amount checks in router

integration-tests/Cargo.toml

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

pallets/route-executor/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = 'pallet-route-executor'
-version = '2.0.2'
+version = '2.1.0'
 description = 'A pallet to execute a route containing a sequence of trades'
 authors = ['GalacticCouncil']
 edition = '2021'

pallets/route-executor/src/lib.rs

@@ -143,6 +143,8 @@ pub mod pallet {
 		RouteUpdateIsNotSuccessful,
 		///Insufficient asset is not supported for on chain routing
 		InsufficientAssetNotSupported,
+		///The route execution failed in the underlying AMM
+		InvalidRouteExecution,
 	}
 
 	/// Storing routes for asset pairs
@@ -190,7 +192,8 @@ pub mod pallet {
 
 			let user_balance_of_asset_in_before_trade =
 				T::Currency::reducible_balance(asset_in, &who, Preservation::Expendable, Fortitude::Polite);
-
+			let user_balance_of_asset_out_before_trade =
+				T::Currency::reducible_balance(asset_out, &who, Preservation::Expendable, Fortitude::Polite);
 			ensure!(
 				user_balance_of_asset_in_before_trade >= amount_in,
 				Error::<T>::InsufficientBalance
@@ -205,6 +208,9 @@ pub mod pallet {
 			);
 
 			for (trade_amount, trade) in trade_amounts.iter().zip(route) {
+				let user_balance_of_asset_in_before_trade =
+					T::Currency::reducible_balance(trade.asset_in, &who, Preservation::Expendable, Fortitude::Polite);
+
 				let execution_result = T::AMM::execute_sell(
 					origin.clone(),
 					trade.pool,
@@ -215,8 +221,22 @@ pub mod pallet {
 				);
 
 				handle_execution_error!(execution_result);
+
+				Self::ensure_that_user_spent_asset_in_at_least(
+					who.clone(),
+					trade.asset_in,
+					user_balance_of_asset_in_before_trade,
+					trade_amount.amount_in,
+				)?;
 			}
 
+			Self::ensure_that_user_received_asset_out_at_most(
+				who,
+				asset_out,
+				user_balance_of_asset_out_before_trade,
+				last_trade_amount.amount_out,
+			)?;
+
 			Self::deposit_event(Event::Executed {
 				asset_in,
 				asset_out,
@@ -251,18 +271,25 @@ pub mod pallet {
 			max_amount_in: T::Balance,
 			route: Vec<Trade<T::AssetId>>,
 		) -> DispatchResult {
+			let who = ensure_signed(origin.clone())?;
+
 			Self::ensure_route_size(route.len())?;
 
 			let asset_pair = AssetPair::new(asset_in, asset_out);
 			let route = Self::get_route_or_default(route, asset_pair)?;
 			Self::ensure_route_arguments(&asset_pair, &route)?;
 
+			let user_balance_of_asset_in_before_trade =
+				T::Currency::reducible_balance(asset_in, &who, Preservation::Expendable, Fortitude::Polite);
+
 			let trade_amounts = Self::calculate_buy_trade_amounts(&route, amount_out)?;
 
 			let first_trade = trade_amounts.last().ok_or(Error::<T>::RouteCalculationFailed)?;
 			ensure!(first_trade.amount_in <= max_amount_in, Error::<T>::TradingLimitReached);
 
 			for (trade_amount, trade) in trade_amounts.iter().rev().zip(route) {
+				let user_balance_of_asset_out_before_trade =
+					T::Currency::reducible_balance(trade.asset_out, &who, Preservation::Expendable, Fortitude::Polite);
 				let execution_result = T::AMM::execute_buy(
 					origin.clone(),
 					trade.pool,
@@ -273,8 +300,22 @@ pub mod pallet {
 				);
 
 				handle_execution_error!(execution_result);
+
+				Self::ensure_that_user_received_asset_out_at_most(
+					who.clone(),
+					trade.asset_out,
+					user_balance_of_asset_out_before_trade,
+					trade_amount.amount_out,
+				)?;
 			}
 
+			Self::ensure_that_user_spent_asset_in_at_least(
+				who,
+				asset_in,
+				user_balance_of_asset_in_before_trade,
+				first_trade.amount_in,
+			)?;
+
 			Self::deposit_event(Event::Executed {
 				asset_in,
 				asset_out,
@@ -410,6 +451,48 @@ impl<T: Config> Pallet<T> {
 		Ok(())
 	}
 
+	fn ensure_that_user_received_asset_out_at_most(
+		who: T::AccountId,
+		asset_out: T::AssetId,
+		user_balance_of_asset_out_before_trade: T::Balance,
+		expected_received_amount: T::Balance,
+	) -> Result<(), DispatchError> {
+		let user_balance_of_asset_out_after_trade =
+			T::Currency::reducible_balance(asset_out, &who, Preservation::Expendable, Fortitude::Polite);
+
+		let actual_received_amount = user_balance_of_asset_out_after_trade
+			.checked_sub(&user_balance_of_asset_out_before_trade)
+			.ok_or(Error::<T>::InvalidRouteExecution)?;
+
+		ensure!(
+			actual_received_amount <= expected_received_amount,
+			Error::<T>::InvalidRouteExecution
+		);
+
+		Ok(())
+	}
+
+	fn ensure_that_user_spent_asset_in_at_least(
+		who: T::AccountId,
+		asset_in: T::AssetId,
+		user_balance_of_asset_in_before_trade: T::Balance,
+		expected_spent_amount: T::Balance,
+	) -> Result<(), DispatchError> {
+		let user_balance_of_asset_in_after_trade =
+			T::Currency::reducible_balance(asset_in, &who, Preservation::Expendable, Fortitude::Polite);
+
+		let actual_spent_amount = user_balance_of_asset_in_before_trade
+			.checked_sub(&user_balance_of_asset_in_after_trade)
+			.ok_or(Error::<T>::InvalidRouteExecution)?;
+
+		ensure!(
+			actual_spent_amount >= expected_spent_amount,
+			Error::<T>::InvalidRouteExecution
+		);
+
+		Ok(())
+	}
+
 	fn get_route_or_default(
 		route: Vec<Trade<T::AssetId>>,
 		asset_pair: AssetPair<T::AssetId>,

pallets/route-executor/src/tests/buy.rs

@@ -465,7 +465,7 @@ fn buy_should_fail_when_called_with_non_signed_origin() {
 			//Act and Assert
 			assert_noop!(
 				Router::buy(RuntimeOrigin::none(), HDX, AUSD, amount_to_buy, limit, trades),
-				DispatchError::Other("Wrong origin")
+				sp_runtime::DispatchError::BadOrigin
 			);
 		});
 }

runtime/hydradx/Cargo.toml

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

runtime/hydradx/src/lib.rs

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

runtime/hydradx/src/weights/route_executor.rs

@@ -18,7 +18,7 @@
 //! Autogenerated weights for `pallet_route_executor`
 //!
 //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
-//! DATE: 2024-02-22, STEPS: `10`, REPEAT: `30`, LOW RANGE: `[]`, HIGH RANGE: `[]`
+//! DATE: 2024-03-05, STEPS: `10`, REPEAT: `30`, LOW RANGE: `[]`, HIGH RANGE: `[]`
 //! WORST CASE MAP SIZE: `1000000`
 //! HOSTNAME: `bench-bot`, CPU: `Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz`
 //! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: 1024
@@ -65,19 +65,21 @@ impl<T: frame_system::Config> pallet_route_executor::weights::WeightInfo for Hyd
 	/// The range of component `c` is `[0, 1]`.
 	fn calculate_and_execute_sell_in_lbp(c: u32, ) -> Weight {
 		// Proof Size summary in bytes:
-		//  Measured:  `3402`
+		//  Measured:  `3436`
 		//  Estimated: `13905`
-		// Minimum execution time: 328_806_000 picoseconds.
-		Weight::from_parts(333_203_198, 13905)
-			// Standard Error: 371_747
-			.saturating_add(Weight::from_parts(49_956_676, 0).saturating_mul(c.into()))
+		// Minimum execution time: 347_725_000 picoseconds.
+		Weight::from_parts(351_096_698, 13905)
+			// Standard Error: 200_537
+			.saturating_add(Weight::from_parts(50_253_676, 0).saturating_mul(c.into()))
 			.saturating_add(T::DbWeight::get().reads(16))
 			.saturating_add(T::DbWeight::get().writes(7))
 	}
 	/// Storage: `LBP::PoolData` (r:1 w:0)
 	/// Proof: `LBP::PoolData` (`max_values`: None, `max_size`: Some(163), added: 2638, mode: `MaxEncodedLen`)
 	/// Storage: `Tokens::Accounts` (r:5 w:5)
 	/// Proof: `Tokens::Accounts` (`max_values`: None, `max_size`: Some(108), added: 2583, mode: `MaxEncodedLen`)
+	/// Storage: `System::Account` (r:3 w:1)
+	/// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`)
 	/// Storage: `Tokens::Locks` (r:1 w:1)
 	/// Proof: `Tokens::Locks` (`max_values`: None, `max_size`: Some(1261), added: 3736, mode: `MaxEncodedLen`)
 	/// Storage: `Duster::AccountBlacklist` (r:2 w:0)
@@ -86,26 +88,24 @@ impl<T: frame_system::Config> pallet_route_executor::weights::WeightInfo for Hyd
 	/// Proof: `AssetRegistry::BannedAssets` (`max_values`: None, `max_size`: Some(20), added: 2495, mode: `MaxEncodedLen`)
 	/// Storage: `AssetRegistry::Assets` (r:2 w:0)
 	/// Proof: `AssetRegistry::Assets` (`max_values`: None, `max_size`: Some(125), added: 2600, mode: `MaxEncodedLen`)
-	/// Storage: `System::Account` (r:3 w:1)
-	/// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`)
 	/// The range of component `c` is `[1, 2]`.
 	/// The range of component `b` is `[0, 1]`.
 	fn calculate_and_execute_buy_in_lbp(c: u32, b: u32, ) -> Weight {
 		// Proof Size summary in bytes:
-		//  Measured:  `1571 + b * (1836 ±0)`
+		//  Measured:  `1604 + b * (1837 ±0)`
 		//  Estimated: `6156 + b * (7749 ±99_524_913_928_918_768)`
-		// Minimum execution time: 75_691_000 picoseconds.
-		Weight::from_parts(76_206_000, 6156)
-			// Standard Error: 581_036
-			.saturating_add(Weight::from_parts(2_286_269, 0).saturating_mul(c.into()))
-			// Standard Error: 1_275_540
-			.saturating_add(Weight::from_parts(259_214_464, 0).saturating_mul(b.into()))
+		// Minimum execution time: 76_051_000 picoseconds.
+		Weight::from_parts(76_610_000, 6156)
+			// Standard Error: 604_305
+			.saturating_add(Weight::from_parts(2_329_553, 0).saturating_mul(c.into()))
+			// Standard Error: 1_326_623
+			.saturating_add(Weight::from_parts(278_434_394, 0).saturating_mul(b.into()))
 			.saturating_add(T::DbWeight::get().reads(3))
 			.saturating_add(T::DbWeight::get().reads((13_u64).saturating_mul(b.into())))
 			.saturating_add(T::DbWeight::get().writes((7_u64).saturating_mul(b.into())))
 			.saturating_add(Weight::from_parts(0, 7749).saturating_mul(b.into()))
 	}
-	/// Storage: `AssetRegistry::Assets` (r:6 w:0)
+	/// Storage: `AssetRegistry::Assets` (r:7 w:0)
 	/// Proof: `AssetRegistry::Assets` (`max_values`: None, `max_size`: Some(125), added: 2600, mode: `MaxEncodedLen`)
 	/// Storage: `Router::Routes` (r:1 w:1)
 	/// Proof: `Router::Routes` (`max_values`: None, `max_size`: Some(90), added: 2565, mode: `MaxEncodedLen`)
@@ -161,11 +161,11 @@ impl<T: frame_system::Config> pallet_route_executor::weights::WeightInfo for Hyd
 	/// Proof: `XYK::ShareToken` (`max_values`: None, `max_size`: Some(52), added: 2527, mode: `MaxEncodedLen`)
 	fn set_route_for_xyk() -> Weight {
 		// Proof Size summary in bytes:
-		//  Measured:  `7137`
+		//  Measured:  `7187`
 		//  Estimated: `42318`
-		// Minimum execution time: 2_156_649_000 picoseconds.
-		Weight::from_parts(2_172_125_000, 42318)
-			.saturating_add(T::DbWeight::get().reads(78))
+		// Minimum execution time: 2_333_578_000 picoseconds.
+		Weight::from_parts(2_348_750_000, 42318)
+			.saturating_add(T::DbWeight::get().reads(79))
 			.saturating_add(T::DbWeight::get().writes(1))
 	}
 }