From 2c0d4389bb9932dbd16eeb159b0f3c46a0be3644 Mon Sep 17 00:00:00 2001 From: dmoka Date: Tue, 6 Aug 2024 12:55:57 +0200 Subject: [PATCH 01/11] add sell all to route excecutor --- pallets/route-executor/src/lib.rs | 104 ++++ pallets/route-executor/src/tests/mod.rs | 1 + pallets/route-executor/src/tests/sell_all.rs | 521 +++++++++++++++++++ traits/src/router.rs | 9 + 4 files changed, 635 insertions(+) create mode 100644 pallets/route-executor/src/tests/sell_all.rs diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index b393bf8b6..61ce438ef 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -463,6 +463,95 @@ pub mod pallet { Self::insert_route(asset_pair, new_route) } + + /// Executes a sell with a series of trades specified in the route. + /// It sells all reducible user balance of `asset_in` + /// The price for each trade is determined by the corresponding AMM. + /// + /// - `origin`: The executor of the trade + /// - `asset_in`: The identifier of the asset to sell + /// - `asset_out`: The identifier of the asset to receive + /// - `min_amount_out`: The minimum amount of `asset_out` to receive. + /// - `route`: Series of [`Trade`] to be executed. A [`Trade`] specifies the asset pair (`asset_in`, `asset_out`) and the AMM (`pool`) in which the trade is executed. + /// If not specified, than the on-chain route is used. + /// If no on-chain is present, then omnipool route is used as default + /// + /// Emits `RouteExecuted` when successful. + /// + #[pallet::call_index(4)] + #[pallet::weight(T::WeightInfo::sell_weight(route))] + #[transactional] + pub fn sell_all( + origin: OriginFor, + asset_in: T::AssetId, + asset_out: T::AssetId, + min_amount_out: T::Balance, + route: Vec>, + ) -> DispatchResult { + let who = ensure_signed(origin.clone())?; + + ensure!(asset_in != asset_out, Error::::NotAllowed); + + 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_out_before_trade = + T::Currency::reducible_balance(asset_out, &who, Preservation::Preserve, Fortitude::Polite); + + let amount_in = T::Currency::reducible_balance(asset_in, &who, Preservation::Expendable, Fortitude::Polite); + let trade_amounts = Self::calculate_sell_trade_amounts(&route, amount_in)?; + + let last_trade_amount = trade_amounts.last().ok_or(crate::pallet::Error::::RouteCalculationFailed)?; + ensure!( + last_trade_amount.amount_out >= min_amount_out, + Error::::TradingLimitReached + ); + + //TODO: add do sell if the signature is simple + 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, + trade.asset_in, + trade.asset_out, + trade_amount.amount_in, + trade_amount.amount_out, + ); + + crate::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_in, + asset_out, + user_balance_of_asset_out_before_trade, + last_trade_amount.amount_out, + )?; + + Self::deposit_event(crate::pallet::Event::Executed { + asset_in, + asset_out, + amount_in, + amount_out: last_trade_amount.amount_out, + }); + + Ok(()) + } + } } @@ -724,6 +813,17 @@ impl RouterT::sell(origin, asset_in, asset_out, amount_in, min_amount_out, route) } + fn sell_all( + origin: T::RuntimeOrigin, + asset_in: T::AssetId, + asset_out: T::AssetId, + min_amount_out: T::Balance, + route: Vec>, + ) -> DispatchResult { + Pallet::::sell_all(origin, asset_in, asset_out, min_amount_out, route) + } + + fn buy( origin: T::RuntimeOrigin, asset_in: T::AssetId, @@ -781,6 +881,10 @@ impl RouterT>) -> sp_runtime::DispatchResult { + Ok(()) + } + fn buy( _origin: T::RuntimeOrigin, _asset_in: T::AssetId, diff --git a/pallets/route-executor/src/tests/mod.rs b/pallets/route-executor/src/tests/mod.rs index 48fb30e0c..82ee1017a 100644 --- a/pallets/route-executor/src/tests/mod.rs +++ b/pallets/route-executor/src/tests/mod.rs @@ -4,3 +4,4 @@ pub mod mock; pub mod sell; pub mod set_route; pub mod spot_price; +pub mod sell_all; diff --git a/pallets/route-executor/src/tests/sell_all.rs b/pallets/route-executor/src/tests/sell_all.rs new file mode 100644 index 000000000..76e8880e4 --- /dev/null +++ b/pallets/route-executor/src/tests/sell_all.rs @@ -0,0 +1,521 @@ +// This file is part of HydraDX. + +// Copyright (C) 2020-2022 Intergalactic, Limited (GIB). +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use crate::tests::mock::*; +use crate::{Error, Event, Trade}; +use frame_support::{assert_noop, assert_ok}; +use hydradx_traits::router::AssetPair; +use hydradx_traits::router::PoolType; +use pretty_assertions::assert_eq; +use sp_runtime::DispatchError::BadOrigin; +use sp_runtime::{DispatchError, TokenError}; +use orml_traits::MultiCurrency; + +//TODO: add integration test for small ED so account is killed and allbalance is sold + +#[test] +fn sell_should_work_when_route_has_single_trade() { + ExtBuilder::default().build().execute_with(|| { + //Arrange + let limit = 5; + + let trades = vec![HDX_AUSD_TRADE_IN_XYK]; + + let alice_balance = Currencies::free_balance(HDX, &ALICE); + + //Act + assert_ok!(Router::sell_all( + RuntimeOrigin::signed(ALICE), + HDX, + AUSD, + limit, + trades + )); + + + //Assert + assert_executed_sell_trades(vec![(PoolType::XYK, alice_balance, HDX, AUSD)]); + expect_events(vec![Event::Executed { + asset_in: HDX, + asset_out: AUSD, + amount_in: alice_balance, + amount_out: XYK_SELL_CALCULATION_RESULT, + } + .into()]); + }); +} + + +#[test] +fn sell_should_work_with_omnipool_when_no_specified_or_onchain_route_exist() { + ExtBuilder::default().build().execute_with(|| { + //Arrange + let limit = 1; + + let alice_balance = Currencies::free_balance(HDX, &ALICE); + + //Act + assert_ok!(Router::sell_all( + RuntimeOrigin::signed(ALICE), + HDX, + AUSD, + limit, + vec![] + )); + + //Assert + assert_executed_sell_trades(vec![(PoolType::Omnipool, alice_balance, HDX, AUSD)]); + expect_events(vec![Event::Executed { + asset_in: HDX, + asset_out: AUSD, + amount_in: alice_balance, + amount_out: OMNIPOOL_SELL_CALCULATION_RESULT, + } + .into()]); + }); +} + +#[test] +fn sell_should_work_when_route_has_single_trade_without_native_balance() { + ExtBuilder::default() + .with_endowed_accounts(vec![(ALICE, KSM, 1000)]) + .build() + .execute_with(|| { + //Arrange + let limit = 5; + + let alice_nonnative_balance = Currencies::free_balance(KSM, &ALICE); + + let trades = vec![Trade { + pool: PoolType::XYK, + asset_in: KSM, + asset_out: AUSD, + }]; + + //Act + assert_ok!(Router::sell_all( + RuntimeOrigin::signed(ALICE), + KSM, + AUSD, + limit, + trades + )); + + //Assert + assert_executed_sell_trades(vec![(PoolType::XYK, alice_nonnative_balance, KSM, AUSD)]); + }); +} + + +#[test] +fn sell_should_work_when_route_has_multiple_trades_with_same_pooltype() { + ExtBuilder::default().build().execute_with(|| { + //Arrange + let alice_native_balance = Currencies::free_balance(HDX, &ALICE); + + let limit = 5; + let trade1 = Trade { + pool: PoolType::XYK, + asset_in: HDX, + asset_out: AUSD, + }; + let trade2 = Trade { + pool: PoolType::XYK, + asset_in: AUSD, + asset_out: MOVR, + }; + let trade3 = Trade { + pool: PoolType::XYK, + asset_in: MOVR, + asset_out: KSM, + }; + let trades = vec![trade1, trade2, trade3]; + + //Act + assert_ok!(Router::sell_all( + RuntimeOrigin::signed(ALICE), + HDX, + KSM, + limit, + trades + )); + + //Assert + assert_executed_sell_trades(vec![ + (PoolType::XYK, alice_native_balance, HDX, AUSD), + (PoolType::XYK, XYK_SELL_CALCULATION_RESULT, AUSD, MOVR), + (PoolType::XYK, XYK_SELL_CALCULATION_RESULT, MOVR, KSM), + ]); + expect_events(vec![Event::Executed { + asset_in: HDX, + asset_out: KSM, + amount_in: alice_native_balance, + amount_out: XYK_SELL_CALCULATION_RESULT, + } + .into()]); + }); +} + +#[test] +fn sell_should_work_when_route_has_multiple_trades_with_different_pool_type() { + ExtBuilder::default().build().execute_with(|| { + //Arrange + let alice_native_balance = Currencies::free_balance(HDX, &ALICE); + + let limit = 1; + let trade1 = Trade { + pool: PoolType::XYK, + asset_in: HDX, + asset_out: MOVR, + }; + let trade2 = Trade { + pool: PoolType::Stableswap(AUSD), + asset_in: MOVR, + asset_out: AUSD, + }; + let trade3 = Trade { + pool: PoolType::Omnipool, + asset_in: AUSD, + asset_out: KSM, + }; + let trades = vec![trade1, trade2, trade3]; + + //Act + assert_ok!(Router::sell_all( + RuntimeOrigin::signed(ALICE), + HDX, + KSM, + limit, + trades + )); + + //Assert + assert_executed_sell_trades(vec![ + (PoolType::XYK, alice_native_balance, HDX, MOVR), + (PoolType::Stableswap(AUSD), XYK_SELL_CALCULATION_RESULT, MOVR, AUSD), + (PoolType::Omnipool, STABLESWAP_SELL_CALCULATION_RESULT, AUSD, KSM), + ]); + + expect_events(vec![Event::Executed { + asset_in: HDX, + asset_out: KSM, + amount_in: alice_native_balance, + amount_out: OMNIPOOL_SELL_CALCULATION_RESULT, + } + .into()]); + }); +} + + + +#[test] +fn sell_should_work_with_onchain_route_when_no_routes_specified() { + ExtBuilder::default().build().execute_with(|| { + //Arrange + let alice_native_balance = Currencies::free_balance(HDX, &ALICE); + let limit = 1; + let trade1 = Trade { + pool: PoolType::XYK, + asset_in: HDX, + asset_out: MOVR, + }; + let trade2 = Trade { + pool: PoolType::Stableswap(AUSD), + asset_in: MOVR, + asset_out: AUSD, + }; + let trade3 = Trade { + pool: PoolType::XYK, + asset_in: AUSD, + asset_out: KSM, + }; + let trades = vec![trade1, trade2, trade3]; + assert_ok!(Router::set_route( + RuntimeOrigin::signed(ALICE), + AssetPair::new(HDX, KSM), + trades, + )); + + //Act + assert_ok!(Router::sell_all( + RuntimeOrigin::signed(ALICE), + HDX, + KSM, + limit, + vec![] + )); + + //Assert + assert_last_executed_sell_trades( + 3, + vec![ + (PoolType::XYK, alice_native_balance, HDX, MOVR), + (PoolType::Stableswap(AUSD), XYK_SELL_CALCULATION_RESULT, MOVR, AUSD), + (PoolType::XYK, STABLESWAP_SELL_CALCULATION_RESULT, AUSD, KSM), + ], + ); + + expect_events(vec![Event::Executed { + asset_in: HDX, + asset_out: KSM, + amount_in: alice_native_balance, + amount_out: XYK_SELL_CALCULATION_RESULT, + } + .into()]); + }); +} + + +#[test] +fn sell_should_work_with_onchain_route_when_onchain_route_present_in_reverse_order() { + ExtBuilder::default() + .with_endowed_accounts(vec![(ALICE, KSM,2000)]) + .build() + .execute_with(|| { + //Arrange + let alice_nonnative_balance = Currencies::free_balance(KSM, &ALICE); + + let limit = 1; + let trade1 = Trade { + pool: PoolType::XYK, + asset_in: HDX, + asset_out: MOVR, + }; + let trade2 = Trade { + pool: PoolType::Stableswap(AUSD), + asset_in: MOVR, + asset_out: AUSD, + }; + let trade3 = Trade { + pool: PoolType::XYK, + asset_in: AUSD, + asset_out: KSM, + }; + let trades = vec![trade1, trade2, trade3]; + assert_ok!(Router::set_route( + RuntimeOrigin::signed(ALICE), + AssetPair::new(HDX, KSM), + trades, + )); + + //Act + //it fails, the amount out is not there after all three trades. + assert_ok!(Router::sell_all( + RuntimeOrigin::signed(ALICE), + KSM, + HDX, + limit, + vec![] + )); + + //Assert + assert_last_executed_sell_trades( + 3, + vec![ + (PoolType::XYK, alice_nonnative_balance, KSM, AUSD), + (PoolType::Stableswap(AUSD), XYK_SELL_CALCULATION_RESULT, AUSD, MOVR), + (PoolType::XYK, STABLESWAP_SELL_CALCULATION_RESULT, MOVR, HDX), + ], + ); + + expect_events(vec![Event::Executed { + asset_in: KSM, + asset_out: HDX, + amount_in: alice_nonnative_balance, + amount_out: XYK_SELL_CALCULATION_RESULT, + } + .into()]); + }); +} + +#[test] +fn sell_should_work_when_first_trade_is_not_supported_in_the_first_pool() { + ExtBuilder::default().build().execute_with(|| { + //Arrange + let alice_native_balance = Currencies::free_balance(HDX, &ALICE); + let limit = 5; + let trade1 = Trade { + pool: PoolType::Stableswap(AUSD), + asset_in: HDX, + asset_out: AUSD, + }; + let trade2 = Trade { + pool: PoolType::XYK, + asset_in: AUSD, + asset_out: KSM, + }; + let trades = vec![trade1, trade2]; + + //Act + assert_ok!(Router::sell_all( + RuntimeOrigin::signed(ALICE), + HDX, + KSM, + limit, + trades + )); + + //Assert + assert_executed_sell_trades(vec![ + (PoolType::Stableswap(AUSD), alice_native_balance, HDX, AUSD), + (PoolType::XYK, STABLESWAP_SELL_CALCULATION_RESULT, AUSD, KSM), + ]); + }); +} + + +#[test] +fn sell_should_fail_when_max_limit_for_trade_reached() { + ExtBuilder::default() + .with_endowed_accounts(vec![(ALICE, HDX, 1000)]) + .build() + .execute_with(|| { + //Arrange + let trade1 = Trade { + pool: PoolType::XYK, + asset_in: HDX, + asset_out: AUSD, + }; + let trade2 = Trade { + pool: PoolType::XYK, + asset_in: AUSD, + asset_out: MOVR, + }; + let trade3 = Trade { + pool: PoolType::XYK, + asset_in: MOVR, + asset_out: KSM, + }; + let trade4 = Trade { + pool: PoolType::XYK, + asset_in: KSM, + asset_out: RMRK, + }; + let trade5 = Trade { + pool: PoolType::XYK, + asset_in: RMRK, + asset_out: SDN, + }; + let trade6 = Trade { + pool: PoolType::XYK, + asset_in: SDN, + asset_out: STABLE_SHARE_ASSET, + }; + let trades = vec![trade1, trade2, trade3, trade4, trade5, trade6]; + + //Act and Assert + assert_noop!( + Router::sell_all(RuntimeOrigin::signed(ALICE), HDX, SDN, 5, trades), + Error::::MaxTradesExceeded + ); + }); +} + + +#[test] +fn sell_should_fail_when_called_with_non_signed_origin() { + ExtBuilder::default().build().execute_with(|| { + //Arrange + let limit = 5; + let trades = vec![HDX_AUSD_TRADE_IN_XYK]; + + //Act and Assert + assert_noop!( + Router::sell_all(RuntimeOrigin::none(), HDX, AUSD, limit, trades), + BadOrigin + ); + }); +} + + +#[test] +fn sell_should_fail_when_min_limit_to_receive_is_not_reached() { + ExtBuilder::default().build().execute_with(|| { + //Arrange + let limit = XYK_SELL_CALCULATION_RESULT + 1; + + let trades = vec![HDX_AUSD_TRADE_IN_XYK]; + + //Act and Assert + assert_noop!( + Router::sell_all(RuntimeOrigin::signed(ALICE), HDX, AUSD, limit, trades), + Error::::TradingLimitReached + ); + }); +} + + +#[test] +fn sell_should_fail_when_assets_dont_correspond_to_route() { + ExtBuilder::default() + .with_endowed_accounts(vec![(ALICE, AUSD, 1000)]) + .build() + .execute_with(|| { + //Arrange + let limit = 5; + + let trades = vec![ + Trade { + pool: PoolType::XYK, + asset_in: AUSD, + asset_out: HDX, + }, + Trade { + pool: PoolType::XYK, + asset_in: HDX, + asset_out: MOVR, + }, + ]; + + //Act and assert + assert_noop!( + Router::sell_all(RuntimeOrigin::signed(ALICE), MOVR, AUSD, limit, trades), + Error::::InvalidRoute + ); + }); +} + + +#[test] +fn sell_should_fail_when_intermediare_assets_are_inconsistent() { + ExtBuilder::default().build().execute_with(|| { + //Arrange + let limit = 5; + let trade1 = Trade { + pool: PoolType::XYK, + asset_in: HDX, + asset_out: AUSD, + }; + let trade2 = Trade { + pool: PoolType::XYK, + asset_in: HDX, + asset_out: MOVR, + }; + let trade3 = Trade { + pool: PoolType::XYK, + asset_in: HDX, + asset_out: KSM, + }; + let trades = vec![trade1, trade2, trade3]; + + //Act + assert_noop!( + Router::sell_all(RuntimeOrigin::signed(ALICE), HDX, KSM, limit, trades), + Error::::InvalidRoute + ); + }); +} diff --git a/traits/src/router.rs b/traits/src/router.rs index 6ded7e27d..515d5bac4 100644 --- a/traits/src/router.rs +++ b/traits/src/router.rs @@ -112,6 +112,15 @@ pub trait RouterT { route: Vec, ) -> DispatchResult; + fn sell_all( + origin: Origin, + asset_in: AssetId, + asset_out: AssetId, + min_amount_out: Balance, + route: Vec, + ) -> DispatchResult; + + fn buy( origin: Origin, asset_in: AssetId, From 983a8b5ca77cfbdd8903769950305e85d1f8a05a Mon Sep 17 00:00:00 2001 From: dmoka Date: Tue, 6 Aug 2024 12:56:22 +0200 Subject: [PATCH 02/11] add integration tests for sell all --- integration-tests/src/router.rs | 88 ++++++++++++++++++++ pallets/route-executor/src/lib.rs | 1 + pallets/route-executor/src/tests/sell_all.rs | 1 - 3 files changed, 89 insertions(+), 1 deletion(-) diff --git a/integration-tests/src/router.rs b/integration-tests/src/router.rs index 943425782..7011ac40c 100644 --- a/integration-tests/src/router.rs +++ b/integration-tests/src/router.rs @@ -4768,6 +4768,94 @@ mod route_spot_price { } } +mod sell_all { + use hydradx_runtime::Currencies; + use super::*; + + #[test] + fn sell_should_sell_all_user_native_balance() { + TestNet::reset(); + + let limit = 0; + let amount_out = 26577363534770086553; + + + Hydra::execute_with(|| { + let bob_hdx_balance = Currencies::free_balance(HDX, &BOB.into()); + + //Arrange + init_omnipool(); + + let trades = vec![Trade { + pool: PoolType::Omnipool, + asset_in: HDX, + asset_out: DAI, + }]; + + //Act + assert_ok!(Router::sell_all( + RuntimeOrigin::signed(BOB.into()), + HDX, + DAI, + limit, + trades + )); + + //Assert + assert_balance!(BOB.into(), HDX, 0); + + expect_hydra_last_events(vec![pallet_route_executor::Event::Executed { + asset_in: HDX, + asset_out: DAI, + amount_in: bob_hdx_balance, + amount_out, + }.into()]); + }); + } + + #[test] + fn sell_all_should_sell_all_user_nonnative_balance() { + TestNet::reset(); + + let limit = 0; + let amount_out = 35227901268414708; + + + Hydra::execute_with(|| { + let bob_nonnative_balance = Currencies::free_balance(DAI, &BOB.into()); + + //Arrange + init_omnipool(); + + let trades = vec![Trade { + pool: PoolType::Omnipool, + asset_in: DAI, + asset_out: HDX, + }]; + + //Act + assert_ok!(Router::sell_all( + RuntimeOrigin::signed(BOB.into()), + DAI, + HDX, + limit, + trades + )); + + //Assert + assert_balance!(BOB.into(), DAI, 0); + + expect_hydra_last_events(vec![pallet_route_executor::Event::Executed { + asset_in: DAI, + asset_out: HDX, + amount_in: bob_nonnative_balance, + amount_out, + }.into()]); + }); + } + +} + fn create_lbp_pool(accumulated_asset: u32, distributed_asset: u32) { assert_ok!(Currencies::update_balance( hydradx_runtime::RuntimeOrigin::root(), diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index 61ce438ef..055ce57b5 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -264,6 +264,7 @@ pub mod pallet { Ok(()) } + /// Executes a buy with a series of trades specified in the route. /// The price for each trade is determined by the corresponding AMM. /// diff --git a/pallets/route-executor/src/tests/sell_all.rs b/pallets/route-executor/src/tests/sell_all.rs index 76e8880e4..5a56ff26c 100644 --- a/pallets/route-executor/src/tests/sell_all.rs +++ b/pallets/route-executor/src/tests/sell_all.rs @@ -22,7 +22,6 @@ use hydradx_traits::router::AssetPair; use hydradx_traits::router::PoolType; use pretty_assertions::assert_eq; use sp_runtime::DispatchError::BadOrigin; -use sp_runtime::{DispatchError, TokenError}; use orml_traits::MultiCurrency; //TODO: add integration test for small ED so account is killed and allbalance is sold From fc3c4ccba8a4e5772c9c484c91a651d95c2dbe38 Mon Sep 17 00:00:00 2001 From: dmoka Date: Tue, 6 Aug 2024 13:07:25 +0200 Subject: [PATCH 03/11] add more test --- integration-tests/src/router.rs | 52 ++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/integration-tests/src/router.rs b/integration-tests/src/router.rs index 7011ac40c..2b2259480 100644 --- a/integration-tests/src/router.rs +++ b/integration-tests/src/router.rs @@ -1340,7 +1340,7 @@ mod omnipool_router_tests { } #[test] - fn sell_should_with_selling_nonnaitve_when_account_providers_increases_during_trade() { + fn sell_should_work_with_selling_nonnaitve_when_account_providers_increases_during_trade() { TestNet::reset(); Hydra::execute_with(|| { @@ -4770,6 +4770,7 @@ mod route_spot_price { mod sell_all { use hydradx_runtime::Currencies; + use hydradx_traits::router::PoolType; use super::*; #[test] @@ -4854,6 +4855,55 @@ mod sell_all { }); } + #[test] + fn sell_all_should_work_when_selling_all_nonnative_in_stableswap() { + TestNet::reset(); + + Hydra::execute_with(|| { + let _ = with_transaction(|| { + //Arrange + let (pool_id, stable_asset_1, _) = init_stableswap().unwrap(); + + init_omnipool(); + + let init_balance = 3000 * UNITS + 1; + assert_ok!(Currencies::update_balance( + hydradx_runtime::RuntimeOrigin::root(), + ALICE.into(), + stable_asset_1, + init_balance as i128, + )); + + let trades = vec![Trade { + pool: PoolType::Stableswap(pool_id), + asset_in: stable_asset_1, + asset_out: pool_id, + }]; + + assert_balance!(ALICE.into(), pool_id, 0); + + //Act + let amount_to_sell = 3000 * UNITS; + assert_ok!(Router::sell_all( + hydradx_runtime::RuntimeOrigin::signed(ALICE.into()), + stable_asset_1, + pool_id, + 0, + trades + )); + + //Assert + assert_eq!( + hydradx_runtime::Currencies::free_balance(stable_asset_1, &AccountId::from(ALICE)), + 0 + ); + + TransactionOutcome::Commit(DispatchResult::Ok(())) + }); + }); + } + + } fn create_lbp_pool(accumulated_asset: u32, distributed_asset: u32) { From a9a3689106398fcea3809f2b9f51716a054777f2 Mon Sep 17 00:00:00 2001 From: dmoka Date: Tue, 6 Aug 2024 14:03:47 +0200 Subject: [PATCH 04/11] add doSell to remove duplication --- pallets/route-executor/src/lib.rs | 177 +++++++++++------------------- 1 file changed, 62 insertions(+), 115 deletions(-) diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index 055ce57b5..72eb4aa7e 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -202,69 +202,9 @@ pub mod pallet { min_amount_out: T::Balance, route: Vec>, ) -> DispatchResult { - let who = ensure_signed(origin.clone())?; - - ensure!(asset_in != asset_out, Error::::NotAllowed); - - 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_out_before_trade = - T::Currency::reducible_balance(asset_out, &who, Preservation::Preserve, Fortitude::Polite); - - let trade_amounts = Self::calculate_sell_trade_amounts(&route, amount_in)?; - - let last_trade_amount = trade_amounts.last().ok_or(Error::::RouteCalculationFailed)?; - ensure!( - last_trade_amount.amount_out >= min_amount_out, - Error::::TradingLimitReached - ); - - 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, - trade.asset_in, - trade.asset_out, - trade_amount.amount_in, - trade_amount.amount_out, - ); - - 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_in, - asset_out, - user_balance_of_asset_out_before_trade, - last_trade_amount.amount_out, - )?; - - Self::deposit_event(Event::Executed { - asset_in, - asset_out, - amount_in, - amount_out: last_trade_amount.amount_out, - }); - - Ok(()) + Self::do_sell(origin, asset_in, asset_out, amount_in,min_amount_out, route) } - /// Executes a buy with a series of trades specified in the route. /// The price for each trade is determined by the corresponding AMM. /// @@ -489,79 +429,85 @@ pub mod pallet { min_amount_out: T::Balance, route: Vec>, ) -> DispatchResult { + //TODO: add proper benchmarking? of since it is one read with cache, it is not a big deal let who = ensure_signed(origin.clone())?; + let amount_in = T::Currency::reducible_balance(asset_in, &who, Preservation::Expendable, Fortitude::Polite); - ensure!(asset_in != asset_out, Error::::NotAllowed); + Self::do_sell(origin, asset_in, asset_out, amount_in, min_amount_out, route) + } - 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)?; +impl Pallet { + /// Pallet account address for do dry-run sell execution as validation + pub fn router_account() -> T::AccountId { + PalletId(*b"routerex").into_account_truncating() + } - let user_balance_of_asset_out_before_trade = - T::Currency::reducible_balance(asset_out, &who, Preservation::Preserve, Fortitude::Polite); + fn do_sell(origin: T::RuntimeOrigin, asset_in: T::AssetId, asset_out: T::AssetId, amount_in: T::Balance, min_amount_out: T::Balance, route: Vec>) -> Result<(), DispatchError> { + let who = ensure_signed(origin.clone())?; + ensure!(asset_in != asset_out, Error::::NotAllowed); - let amount_in = T::Currency::reducible_balance(asset_in, &who, Preservation::Expendable, Fortitude::Polite); - let trade_amounts = Self::calculate_sell_trade_amounts(&route, amount_in)?; + Self::ensure_route_size(route.len())?; - let last_trade_amount = trade_amounts.last().ok_or(crate::pallet::Error::::RouteCalculationFailed)?; - ensure!( + 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_out_before_trade = + T::Currency::reducible_balance(asset_out, &who, Preservation::Preserve, Fortitude::Polite); + + let trade_amounts = Self::calculate_sell_trade_amounts(&route, amount_in)?; + + let last_trade_amount = trade_amounts.last().ok_or(Error::::RouteCalculationFailed)?; + ensure!( last_trade_amount.amount_out >= min_amount_out, Error::::TradingLimitReached ); - //TODO: add do sell if the signature is simple - 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, - trade.asset_in, - trade.asset_out, - trade_amount.amount_in, - trade_amount.amount_out, - ); - - crate::handle_execution_error!(execution_result); + 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, + trade.asset_in, + trade.asset_out, + trade_amount.amount_in, + trade_amount.amount_out, + ); - 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, - )?; - } + crate::handle_execution_error!(execution_result); - Self::ensure_that_user_received_asset_out_at_most( - who, - asset_in, - asset_out, - user_balance_of_asset_out_before_trade, - last_trade_amount.amount_out, + 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::deposit_event(crate::pallet::Event::Executed { - asset_in, - asset_out, - amount_in, - amount_out: last_trade_amount.amount_out, - }); - - Ok(()) } - } -} + Self::ensure_that_user_received_asset_out_at_most( + who, + asset_in, + asset_out, + user_balance_of_asset_out_before_trade, + last_trade_amount.amount_out, + )?; + + Self::deposit_event(Event::Executed { + asset_in, + asset_out, + amount_in, + amount_out: last_trade_amount.amount_out, + }); -impl Pallet { - /// Pallet account address for do dry-run sell execution as validation - pub fn router_account() -> T::AccountId { - PalletId(*b"routerex").into_account_truncating() + Ok(()) } + fn ensure_route_size(route_length: usize) -> Result<(), DispatchError> { ensure!( (route_length as u32) <= MAX_NUMBER_OF_TRADES, @@ -963,6 +909,7 @@ macro_rules! handle_execution_error { } impl RouteProvider for Pallet { + fn get_route(asset_pair: AssetPair) -> Vec> { let onchain_route = Routes::::get(asset_pair.ordered_pair()); From 1d0662e643f2caf609730f28b1c613bcec17ba9b Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 7 Aug 2024 10:21:50 +0200 Subject: [PATCH 05/11] formatting --- integration-tests/src/router.rs | 12 ++- pallets/route-executor/src/lib.rs | 29 ++++--- pallets/route-executor/src/tests/mod.rs | 2 +- pallets/route-executor/src/tests/sell_all.rs | 79 +++----------------- traits/src/router.rs | 1 - 5 files changed, 35 insertions(+), 88 deletions(-) diff --git a/integration-tests/src/router.rs b/integration-tests/src/router.rs index 2b2259480..63f432524 100644 --- a/integration-tests/src/router.rs +++ b/integration-tests/src/router.rs @@ -4769,9 +4769,9 @@ mod route_spot_price { } mod sell_all { + use super::*; use hydradx_runtime::Currencies; use hydradx_traits::router::PoolType; - use super::*; #[test] fn sell_should_sell_all_user_native_balance() { @@ -4780,7 +4780,6 @@ mod sell_all { let limit = 0; let amount_out = 26577363534770086553; - Hydra::execute_with(|| { let bob_hdx_balance = Currencies::free_balance(HDX, &BOB.into()); @@ -4810,7 +4809,8 @@ mod sell_all { asset_out: DAI, amount_in: bob_hdx_balance, amount_out, - }.into()]); + } + .into()]); }); } @@ -4821,7 +4821,6 @@ mod sell_all { let limit = 0; let amount_out = 35227901268414708; - Hydra::execute_with(|| { let bob_nonnative_balance = Currencies::free_balance(DAI, &BOB.into()); @@ -4851,7 +4850,8 @@ mod sell_all { asset_out: HDX, amount_in: bob_nonnative_balance, amount_out, - }.into()]); + } + .into()]); }); } @@ -4902,8 +4902,6 @@ mod sell_all { }); }); } - - } fn create_lbp_pool(accumulated_asset: u32, distributed_asset: u32) { diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index 72eb4aa7e..b18ddb81b 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -202,7 +202,7 @@ pub mod pallet { min_amount_out: T::Balance, route: Vec>, ) -> DispatchResult { - Self::do_sell(origin, asset_in, asset_out, amount_in,min_amount_out, route) + Self::do_sell(origin, asset_in, asset_out, amount_in, min_amount_out, route) } /// Executes a buy with a series of trades specified in the route. @@ -435,7 +435,6 @@ pub mod pallet { Self::do_sell(origin, asset_in, asset_out, amount_in, min_amount_out, route) } - } } @@ -445,7 +444,14 @@ impl Pallet { PalletId(*b"routerex").into_account_truncating() } - fn do_sell(origin: T::RuntimeOrigin, asset_in: T::AssetId, asset_out: T::AssetId, amount_in: T::Balance, min_amount_out: T::Balance, route: Vec>) -> Result<(), DispatchError> { + fn do_sell( + origin: T::RuntimeOrigin, + asset_in: T::AssetId, + asset_out: T::AssetId, + amount_in: T::Balance, + min_amount_out: T::Balance, + route: Vec>, + ) -> Result<(), DispatchError> { let who = ensure_signed(origin.clone())?; ensure!(asset_in != asset_out, Error::::NotAllowed); @@ -462,9 +468,9 @@ impl Pallet { let last_trade_amount = trade_amounts.last().ok_or(Error::::RouteCalculationFailed)?; ensure!( - last_trade_amount.amount_out >= min_amount_out, - Error::::TradingLimitReached - ); + last_trade_amount.amount_out >= min_amount_out, + Error::::TradingLimitReached + ); for (trade_amount, trade) in trade_amounts.iter().zip(route) { let user_balance_of_asset_in_before_trade = @@ -507,7 +513,6 @@ impl Pallet { Ok(()) } - fn ensure_route_size(route_length: usize) -> Result<(), DispatchError> { ensure!( (route_length as u32) <= MAX_NUMBER_OF_TRADES, @@ -770,7 +775,6 @@ impl RouterT::sell_all(origin, asset_in, asset_out, min_amount_out, route) } - fn buy( origin: T::RuntimeOrigin, asset_in: T::AssetId, @@ -828,7 +832,13 @@ impl RouterT>) -> sp_runtime::DispatchResult { + fn sell_all( + _origin: T::RuntimeOrigin, + _asset_in: T::AssetId, + _asset_out: T::AssetId, + _min_amount_out: T::Balance, + _route: Vec>, + ) -> sp_runtime::DispatchResult { Ok(()) } @@ -909,7 +919,6 @@ macro_rules! handle_execution_error { } impl RouteProvider for Pallet { - fn get_route(asset_pair: AssetPair) -> Vec> { let onchain_route = Routes::::get(asset_pair.ordered_pair()); diff --git a/pallets/route-executor/src/tests/mod.rs b/pallets/route-executor/src/tests/mod.rs index 82ee1017a..735e9dabe 100644 --- a/pallets/route-executor/src/tests/mod.rs +++ b/pallets/route-executor/src/tests/mod.rs @@ -2,6 +2,6 @@ pub mod buy; pub mod force_insert_route; pub mod mock; pub mod sell; +pub mod sell_all; pub mod set_route; pub mod spot_price; -pub mod sell_all; diff --git a/pallets/route-executor/src/tests/sell_all.rs b/pallets/route-executor/src/tests/sell_all.rs index 5a56ff26c..d34f638cf 100644 --- a/pallets/route-executor/src/tests/sell_all.rs +++ b/pallets/route-executor/src/tests/sell_all.rs @@ -20,9 +20,9 @@ use crate::{Error, Event, Trade}; use frame_support::{assert_noop, assert_ok}; use hydradx_traits::router::AssetPair; use hydradx_traits::router::PoolType; +use orml_traits::MultiCurrency; use pretty_assertions::assert_eq; use sp_runtime::DispatchError::BadOrigin; -use orml_traits::MultiCurrency; //TODO: add integration test for small ED so account is killed and allbalance is sold @@ -37,14 +37,7 @@ fn sell_should_work_when_route_has_single_trade() { let alice_balance = Currencies::free_balance(HDX, &ALICE); //Act - assert_ok!(Router::sell_all( - RuntimeOrigin::signed(ALICE), - HDX, - AUSD, - limit, - trades - )); - + assert_ok!(Router::sell_all(RuntimeOrigin::signed(ALICE), HDX, AUSD, limit, trades)); //Assert assert_executed_sell_trades(vec![(PoolType::XYK, alice_balance, HDX, AUSD)]); @@ -58,7 +51,6 @@ fn sell_should_work_when_route_has_single_trade() { }); } - #[test] fn sell_should_work_with_omnipool_when_no_specified_or_onchain_route_exist() { ExtBuilder::default().build().execute_with(|| { @@ -68,13 +60,7 @@ fn sell_should_work_with_omnipool_when_no_specified_or_onchain_route_exist() { let alice_balance = Currencies::free_balance(HDX, &ALICE); //Act - assert_ok!(Router::sell_all( - RuntimeOrigin::signed(ALICE), - HDX, - AUSD, - limit, - vec![] - )); + assert_ok!(Router::sell_all(RuntimeOrigin::signed(ALICE), HDX, AUSD, limit, vec![])); //Assert assert_executed_sell_trades(vec![(PoolType::Omnipool, alice_balance, HDX, AUSD)]); @@ -106,20 +92,13 @@ fn sell_should_work_when_route_has_single_trade_without_native_balance() { }]; //Act - assert_ok!(Router::sell_all( - RuntimeOrigin::signed(ALICE), - KSM, - AUSD, - limit, - trades - )); + assert_ok!(Router::sell_all(RuntimeOrigin::signed(ALICE), KSM, AUSD, limit, trades)); //Assert assert_executed_sell_trades(vec![(PoolType::XYK, alice_nonnative_balance, KSM, AUSD)]); }); } - #[test] fn sell_should_work_when_route_has_multiple_trades_with_same_pooltype() { ExtBuilder::default().build().execute_with(|| { @@ -145,13 +124,7 @@ fn sell_should_work_when_route_has_multiple_trades_with_same_pooltype() { let trades = vec![trade1, trade2, trade3]; //Act - assert_ok!(Router::sell_all( - RuntimeOrigin::signed(ALICE), - HDX, - KSM, - limit, - trades - )); + assert_ok!(Router::sell_all(RuntimeOrigin::signed(ALICE), HDX, KSM, limit, trades)); //Assert assert_executed_sell_trades(vec![ @@ -194,13 +167,7 @@ fn sell_should_work_when_route_has_multiple_trades_with_different_pool_type() { let trades = vec![trade1, trade2, trade3]; //Act - assert_ok!(Router::sell_all( - RuntimeOrigin::signed(ALICE), - HDX, - KSM, - limit, - trades - )); + assert_ok!(Router::sell_all(RuntimeOrigin::signed(ALICE), HDX, KSM, limit, trades)); //Assert assert_executed_sell_trades(vec![ @@ -219,8 +186,6 @@ fn sell_should_work_when_route_has_multiple_trades_with_different_pool_type() { }); } - - #[test] fn sell_should_work_with_onchain_route_when_no_routes_specified() { ExtBuilder::default().build().execute_with(|| { @@ -250,13 +215,7 @@ fn sell_should_work_with_onchain_route_when_no_routes_specified() { )); //Act - assert_ok!(Router::sell_all( - RuntimeOrigin::signed(ALICE), - HDX, - KSM, - limit, - vec![] - )); + assert_ok!(Router::sell_all(RuntimeOrigin::signed(ALICE), HDX, KSM, limit, vec![])); //Assert assert_last_executed_sell_trades( @@ -278,11 +237,10 @@ fn sell_should_work_with_onchain_route_when_no_routes_specified() { }); } - #[test] fn sell_should_work_with_onchain_route_when_onchain_route_present_in_reverse_order() { ExtBuilder::default() - .with_endowed_accounts(vec![(ALICE, KSM,2000)]) + .with_endowed_accounts(vec![(ALICE, KSM, 2000)]) .build() .execute_with(|| { //Arrange @@ -313,13 +271,7 @@ fn sell_should_work_with_onchain_route_when_onchain_route_present_in_reverse_ord //Act //it fails, the amount out is not there after all three trades. - assert_ok!(Router::sell_all( - RuntimeOrigin::signed(ALICE), - KSM, - HDX, - limit, - vec![] - )); + assert_ok!(Router::sell_all(RuntimeOrigin::signed(ALICE), KSM, HDX, limit, vec![])); //Assert assert_last_executed_sell_trades( @@ -360,13 +312,7 @@ fn sell_should_work_when_first_trade_is_not_supported_in_the_first_pool() { let trades = vec![trade1, trade2]; //Act - assert_ok!(Router::sell_all( - RuntimeOrigin::signed(ALICE), - HDX, - KSM, - limit, - trades - )); + assert_ok!(Router::sell_all(RuntimeOrigin::signed(ALICE), HDX, KSM, limit, trades)); //Assert assert_executed_sell_trades(vec![ @@ -376,7 +322,6 @@ fn sell_should_work_when_first_trade_is_not_supported_in_the_first_pool() { }); } - #[test] fn sell_should_fail_when_max_limit_for_trade_reached() { ExtBuilder::default() @@ -424,7 +369,6 @@ fn sell_should_fail_when_max_limit_for_trade_reached() { }); } - #[test] fn sell_should_fail_when_called_with_non_signed_origin() { ExtBuilder::default().build().execute_with(|| { @@ -440,7 +384,6 @@ fn sell_should_fail_when_called_with_non_signed_origin() { }); } - #[test] fn sell_should_fail_when_min_limit_to_receive_is_not_reached() { ExtBuilder::default().build().execute_with(|| { @@ -457,7 +400,6 @@ fn sell_should_fail_when_min_limit_to_receive_is_not_reached() { }); } - #[test] fn sell_should_fail_when_assets_dont_correspond_to_route() { ExtBuilder::default() @@ -488,7 +430,6 @@ fn sell_should_fail_when_assets_dont_correspond_to_route() { }); } - #[test] fn sell_should_fail_when_intermediare_assets_are_inconsistent() { ExtBuilder::default().build().execute_with(|| { diff --git a/traits/src/router.rs b/traits/src/router.rs index 515d5bac4..e8d6d615b 100644 --- a/traits/src/router.rs +++ b/traits/src/router.rs @@ -120,7 +120,6 @@ pub trait RouterT { route: Vec, ) -> DispatchResult; - fn buy( origin: Origin, asset_in: AssetId, From 163d9141c984f8d436cf5c92ef1a82899662e50a Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 7 Aug 2024 10:22:31 +0200 Subject: [PATCH 06/11] remove todo as we dont need to process --- pallets/route-executor/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index b18ddb81b..ffd1f1b6d 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -429,7 +429,6 @@ pub mod pallet { min_amount_out: T::Balance, route: Vec>, ) -> DispatchResult { - //TODO: add proper benchmarking? of since it is one read with cache, it is not a big deal let who = ensure_signed(origin.clone())?; let amount_in = T::Currency::reducible_balance(asset_in, &who, Preservation::Expendable, Fortitude::Polite); From 1dbfcbbb827d60029b8ee1c3fb5a5a71867cd0b8 Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 7 Aug 2024 10:23:17 +0200 Subject: [PATCH 07/11] fix typo --- pallets/route-executor/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/route-executor/README.md b/pallets/route-executor/README.md index 09ad44f81..4a05b9e79 100644 --- a/pallets/route-executor/README.md +++ b/pallets/route-executor/README.md @@ -14,7 +14,7 @@ The comparison happens by calculating sell amount_outs for the routes, but also The route is stored in an ordered manner, based on the oder of the ids in the asset pair. -If the route is set successfully, then the fee is payed back. +If the route is set successfully, then the fee is paid back. If the route setting fails, it emits event `RouteUpdateIsNotSuccessful` From 6befe6b33d8b2b6d2c423f489b2975925799075b Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 7 Aug 2024 10:24:16 +0200 Subject: [PATCH 08/11] update doc --- pallets/route-executor/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pallets/route-executor/README.md b/pallets/route-executor/README.md index 4a05b9e79..d732cbd27 100644 --- a/pallets/route-executor/README.md +++ b/pallets/route-executor/README.md @@ -36,5 +36,7 @@ If not on-chain is present, then omnipool is used as default Both buy and sell trades are supported. +There is also a `sell_all` extrinsic, which sells all the reducible `asset_in` balance of the user. + ### Weight calculation The extrinsic weights are calculated based on the size of the route. From 7531faea2f375015691f8a63fded19e27a60fb23 Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 7 Aug 2024 10:24:45 +0200 Subject: [PATCH 09/11] delete done todo --- pallets/route-executor/src/tests/sell_all.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/pallets/route-executor/src/tests/sell_all.rs b/pallets/route-executor/src/tests/sell_all.rs index d34f638cf..a95700ae4 100644 --- a/pallets/route-executor/src/tests/sell_all.rs +++ b/pallets/route-executor/src/tests/sell_all.rs @@ -24,8 +24,6 @@ use orml_traits::MultiCurrency; use pretty_assertions::assert_eq; use sp_runtime::DispatchError::BadOrigin; -//TODO: add integration test for small ED so account is killed and allbalance is sold - #[test] fn sell_should_work_when_route_has_single_trade() { ExtBuilder::default().build().execute_with(|| { From 2d6471a8b4875dad00a59b89b6b3d1ce711880b1 Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 21 Aug 2024 10:18:23 +0200 Subject: [PATCH 10/11] formatting --- pallets/route-executor/src/lib.rs | 102 +++++++++++++++--------------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index 7994a9b31..ac64928dd 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -462,71 +462,71 @@ impl Pallet { min_amount_out: T::Balance, route: Vec>, ) -> Result<(), DispatchError> { - let who = ensure_signed(origin.clone())?; - - ensure!(asset_in != asset_out, Error::::NotAllowed); + let who = ensure_signed(origin.clone())?; - Self::ensure_route_size(route.len())?; + ensure!(asset_in != asset_out, Error::::NotAllowed); - 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)?; + Self::ensure_route_size(route.len())?; - let user_balance_of_asset_out_before_trade = - T::Currency::reducible_balance(asset_out, &who, Preservation::Preserve, Fortitude::Polite); + 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 trade_amounts = Self::calculate_sell_trade_amounts(&route, amount_in)?; + let user_balance_of_asset_out_before_trade = + T::Currency::reducible_balance(asset_out, &who, Preservation::Preserve, Fortitude::Polite); - let last_trade_amount = trade_amounts.last().ok_or(Error::::RouteCalculationFailed)?; - ensure!( - last_trade_amount.amount_out >= min_amount_out, - Error::::TradingLimitReached - ); + let trade_amounts = Self::calculate_sell_trade_amounts(&route, amount_in)?; - let route_length = route.len(); - 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); + let last_trade_amount = trade_amounts.last().ok_or(Error::::RouteCalculationFailed)?; + ensure!( + last_trade_amount.amount_out >= min_amount_out, + Error::::TradingLimitReached + ); - let user_balance_of_asset_in_before_trade = - T::Currency::reducible_balance(trade.asset_in, &who, Preservation::Expendable, Fortitude::Polite); + let route_length = route.len(); + 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); - let execution_result = T::AMM::execute_sell( - origin.clone(), - trade.pool, - trade.asset_in, - trade.asset_out, - trade_amount.amount_in, - trade_amount.amount_out, - ); + 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, + trade.asset_in, + trade.asset_out, + trade_amount.amount_in, + trade_amount.amount_out, + ); - handle_execution_error!(execution_result); + 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_spent_asset_in_at_least( + who.clone(), + trade.asset_in, + user_balance_of_asset_in_before_trade, + trade_amount.amount_in, + )?; + } - SkipEd::::kill(); + SkipEd::::kill(); - Self::ensure_that_user_received_asset_out_at_most( - who, - asset_in, - asset_out, - user_balance_of_asset_out_before_trade, - last_trade_amount.amount_out, - )?; + Self::ensure_that_user_received_asset_out_at_most( + who, + asset_in, + asset_out, + user_balance_of_asset_out_before_trade, + last_trade_amount.amount_out, + )?; - Self::deposit_event(Event::Executed { - asset_in, - asset_out, - amount_in, - amount_out: last_trade_amount.amount_out, - }); + Self::deposit_event(Event::Executed { + asset_in, + asset_out, + amount_in, + amount_out: last_trade_amount.amount_out, + }); - Ok(()) + Ok(()) } fn ensure_route_size(route_length: usize) -> Result<(), DispatchError> { From 837eac7214e20ee1f7b9c477b456c9f3be636597 Mon Sep 17 00:00:00 2001 From: dmoka Date: Fri, 23 Aug 2024 08:12:24 +0200 Subject: [PATCH 11/11] bump versions --- Cargo.lock | 6 +++--- integration-tests/Cargo.toml | 2 +- pallets/route-executor/Cargo.toml | 2 +- traits/Cargo.toml | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 12035b11c..d9baf8680 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5073,7 +5073,7 @@ dependencies = [ [[package]] name = "hydradx-traits" -version = "3.4.0" +version = "3.5.0" dependencies = [ "frame-support", "impl-trait-for-tuples", @@ -8936,7 +8936,7 @@ dependencies = [ [[package]] name = "pallet-route-executor" -version = "2.5.1" +version = "2.6.0" dependencies = [ "frame-benchmarking", "frame-support", @@ -11929,7 +11929,7 @@ dependencies = [ [[package]] name = "runtime-integration-tests" -version = "1.23.2" +version = "1.23.3" dependencies = [ "cumulus-pallet-aura-ext", "cumulus-pallet-parachain-system", diff --git a/integration-tests/Cargo.toml b/integration-tests/Cargo.toml index 15b8ee223..2d816ba0a 100644 --- a/integration-tests/Cargo.toml +++ b/integration-tests/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "runtime-integration-tests" -version = "1.23.2" +version = "1.23.3" description = "Integration tests" authors = ["GalacticCouncil"] edition = "2021" diff --git a/pallets/route-executor/Cargo.toml b/pallets/route-executor/Cargo.toml index 839ef0e51..16894edfd 100644 --- a/pallets/route-executor/Cargo.toml +++ b/pallets/route-executor/Cargo.toml @@ -1,6 +1,6 @@ [package] name = 'pallet-route-executor' -version = '2.5.1' +version = '2.6.0' description = 'A pallet to execute a route containing a sequence of trades' authors = ['GalacticCouncil'] edition = '2021' diff --git a/traits/Cargo.toml b/traits/Cargo.toml index d162a8230..2a08d0e31 100644 --- a/traits/Cargo.toml +++ b/traits/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "hydradx-traits" -version = "3.4.0" +version = "3.5.0" description = "Shared traits" authors = ["GalacticCouncil"] edition = "2021"