diff --git a/Cargo.lock b/Cargo.lock index 1eccf4ce9..2f30161a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4938,7 +4938,7 @@ dependencies = [ [[package]] name = "hydradx-runtime" -version = "254.0.0" +version = "255.0.0" dependencies = [ "cumulus-pallet-aura-ext", "cumulus-pallet-parachain-system", @@ -8768,7 +8768,7 @@ dependencies = [ [[package]] name = "pallet-otc-settlements" -version = "1.0.2" +version = "1.0.3" dependencies = [ "frame-benchmarking", "frame-support", diff --git a/pallets/otc-settlements/Cargo.toml b/pallets/otc-settlements/Cargo.toml index bfc59eae6..24e23f5d2 100644 --- a/pallets/otc-settlements/Cargo.toml +++ b/pallets/otc-settlements/Cargo.toml @@ -1,6 +1,6 @@ [package] name = 'pallet-otc-settlements' -version = '1.0.2' +version = '1.0.3' description = 'A pallet with offchain worker closing OTC arbs' authors = ['GalacticCouncil'] edition = '2021' diff --git a/pallets/otc-settlements/README.md b/pallets/otc-settlements/README.md index 91b5364f9..080e2d109 100644 --- a/pallets/otc-settlements/README.md +++ b/pallets/otc-settlements/README.md @@ -9,7 +9,8 @@ by any user using signed origin. In the former case, the block producer doesn't ## Notes If the OTC order is partially fillable, the pallet tries to close the arbitrage opportunity by finding the amount that aligns the OTC and the Omnipool prices. Executing this trade needs to be profitable, but we are not trying to maximize -the profit. +the profit. If the pallet couldn't find the amount that closes the arb, the amount that reduces the size of the arb is +used. In the case of not partially fillable OTC orders, the pallet tries to maximize the profit. ## Dispatachable functions diff --git a/pallets/otc-settlements/src/lib.rs b/pallets/otc-settlements/src/lib.rs index aed99cd45..faa9a3538 100644 --- a/pallets/otc-settlements/src/lib.rs +++ b/pallets/otc-settlements/src/lib.rs @@ -25,7 +25,7 @@ //! ## Notes //! If the OTC order is partially fillable, the pallet tries to close the arbitrage opportunity by finding the amount that //! aligns the OTC and the Omnipool prices. Executing this trade needs to be profitable, but we are not trying to maximize -//! the profit. +//! the profit. If the pallet couldn't find the amount that closes the arb, the amount that reduces the size of the arb is used. //! In the case of not partially fillable OTC orders, the pallet tries to maximize the profit. //! //! ## Dispatachable functions @@ -33,18 +33,20 @@ #![cfg_attr(not(feature = "std"), no_std)] +#[cfg(not(feature = "runtime-benchmarks"))] +use frame_system::RawOrigin; + use frame_support::{ pallet_prelude::*, traits::{ fungibles::{Inspect, Mutate}, tokens::{Fortitude, Precision, Preservation}, }, - PalletId, + transactional, PalletId, }; use frame_system::{ offchain::{SendTransactionTypes, SubmitTransaction}, pallet_prelude::{BlockNumberFor, OriginFor}, - RawOrigin, }; use hydradx_traits::router::{ AmmTradeWeights, AmountInAndOut, AssetPair, RouteProvider, RouteSpotPriceProvider, RouterT, Trade, @@ -221,7 +223,7 @@ pub mod pallet { /// /// Executes a trade between an OTC order and some route. /// If the OTC order is partially fillable, the extrinsic fails if the existing arbitrage - /// opportunity is not closed after the trade. + /// opportunity is not closed or reduced after the trade. /// If the OTC order is not partially fillable, fails if there is no profit after the trade. /// /// `Origin` calling this extrinsic is not paying or receiving anything. @@ -250,7 +252,9 @@ pub mod pallet { amount: Balance, route: Vec>>, ) -> DispatchResult { - Self::settle_otc(otc_id, amount, route) + // `is_execution` is set to `true`, so both full and partial closing of arbs is allowed. + // If set to `false`, an arb needs to be fully closed. + Self::settle_otc(otc_id, amount, route, true) } } } @@ -280,15 +284,23 @@ impl Pallet { /// /// Executes two trades: asset_a -> OTC -> asset_b, and asset_b -> Router -> asset_a. /// - /// If the OTC order is partially fillable, the extrinsic fails if the existing arbitrage + /// If the OTC order is partially fillable and `is_execution` is set to `false`, the extrinsic fails if the existing arbitrage /// opportunity is not closed after the trade and if there is no profit after the trade. + /// If the OTC order is partially fillable and `is_execution` is set to `true`, arbs are allowed to be partially closed. /// If the OTC order is not partially fillable, fails only if there is no profit after the trade. /// /// Parameters: /// - `otc_id`: ID of the OTC order with existing arbitrage opportunity. /// - `amount`: Amount necessary to close the arb. /// - `route`: The route we trade against. Required for the fee calculation. - pub fn settle_otc(otc_id: OrderId, amount: Balance, route: Vec>>) -> DispatchResult { + /// - `is_execution`: When enabled, test for the price precision is disabled. + #[transactional] + pub fn settle_otc( + otc_id: OrderId, + amount: Balance, + route: Vec>>, + is_execution: bool, + ) -> DispatchResult { log::debug!( target: "offchain_worker::settle_otc", "calling settle_otc(): otc_id: {:?} amount: {:?} route: {:?}", otc_id, amount, route); @@ -368,7 +380,7 @@ impl Pallet { // Compare OTC and Router price. // In the case of fully fillable orders, the resulting price is not important. - if otc.partially_fillable { + if !is_execution && otc.partially_fillable { let price_diff = { if otc_price > router_price_after { otc_price.saturating_sub(router_price_after) @@ -382,7 +394,10 @@ impl Pallet { .ok_or(ArithmeticError::Overflow)?; if price_diff > price_precision { ensure!(router_price_after <= otc_price, Error::::TradeAmountTooHigh); - ensure!(router_price_after >= otc_price, Error::::TradeAmountTooLow); + // partially fillable OTC can be fully filled if the arb is reduced + if amount != otc.amount_in { + ensure!(router_price_after >= otc_price, Error::::TradeAmountTooLow); + } } } @@ -559,7 +574,7 @@ impl Pallet { log::debug!( target: "offchain_worker::settle_otcs::binary_search", "\nsell_amt: {:?}\nsell_amt_up: {:?}\nsell_amt_down: {:?}", sell_amt, sell_amt_up, sell_amt_down); - match Self::settle_otc_order(RawOrigin::None.into(), otc_id, sell_amt, route.to_vec()) { + match Self::settle_otc(otc_id, sell_amt, route.to_vec(), false) { Ok(_) => { log::debug!( target: "offchain_worker::settle_otcs", @@ -590,12 +605,21 @@ impl Pallet { } } } + // no more values to test if sell_amt_down == sell_amt_up { - return None; + break; } sell_amt = (sell_amt_up.checked_add(sell_amt_down)).and_then(|value| value.checked_div(2))?; } - None + // execute with the latest min value + if sell_amt_down != T::MinTradingLimit::get() { + match Self::settle_otc(otc_id, sell_amt_down, route.to_vec(), true) { + Ok(_) => Some(sell_amt_down), + Err(_) => None, + } + } else { + None + } } /// Calculates the price (asset_out/asset_in) after subtracting the OTC fee from the amount_out. diff --git a/pallets/otc-settlements/src/mock.rs b/pallets/otc-settlements/src/mock.rs index 50b7bc7c9..0fb3789c9 100644 --- a/pallets/otc-settlements/src/mock.rs +++ b/pallets/otc-settlements/src/mock.rs @@ -414,7 +414,7 @@ impl Default for ExtBuilder { endowed_accounts: vec![ (ALICE, HDX, 1_000_000_000_000 * ONE), (ALICE, LRNA, 1_000_000_000_000 * ONE), - (ALICE, DAI, 1_000_000_000_000 * ONE), + (ALICE, DAI, 1_000_000_000_000_000_000 * ONE), (ALICE, DOT, 1_000_000_000_000 * ONE), (ALICE, KSM, 1_000_000_000_000 * ONE), (ALICE, BTC, 1_000_000_000_000 * ONE), diff --git a/pallets/otc-settlements/src/tests.rs b/pallets/otc-settlements/src/tests.rs index 2365a88da..fde983164 100644 --- a/pallets/otc-settlements/src/tests.rs +++ b/pallets/otc-settlements/src/tests.rs @@ -250,7 +250,7 @@ fn profit_should_be_transferred_to_treasury_when_nonzero_initial_pallet_balance( } #[test] -fn existing_arb_opportunity_should_trigger_trade() { +fn existing_arb_opportunity_should_trigger_trade_when_correct_amount_can_be_found() { let (mut ext, _) = ExtBuilder::default().build(); ext.execute_with(|| { assert_ok!(OTC::place_order( @@ -300,6 +300,62 @@ fn existing_arb_opportunity_should_trigger_trade() { }); } +#[test] +fn existing_arb_opportunity_should_trigger_trade_when_partially_fillable_otc_can_be_fully_filled() { + let (mut ext, _) = ExtBuilder::default().build(); + ext.execute_with(|| { + assert_ok!(OTC::place_order( + RuntimeOrigin::signed(ALICE), + HDX, // otc asset_in + DAI, // otc asset_out + 100 * ONE, + 205 * ONE, + true, + )); + + // get otc price + let otc_id = 0; + let otc = >::get(otc_id).unwrap(); + let otc_price = calculate_otc_price(&otc); + + // get trade price + let route = Router::get_route(AssetPair { + asset_in: otc.asset_out, + asset_out: otc.asset_in, + }); + let initial_router_price = Router::spot_price_with_fee(&route).unwrap(); + + // verify that there's an arb opportunity + assert!(otc_price > initial_router_price); + + let hdx_total_issuance = Currencies::total_issuance(HDX); + let dai_total_issuance = Currencies::total_issuance(DAI); + + assert!(Currencies::free_balance(HDX, &OtcSettlements::account_id()) == 0); + assert!(Currencies::free_balance(DAI, &OtcSettlements::account_id()) == 0); + + >>::offchain_worker(System::block_number()); + + // verify that the arb is still there, but smaller than before + let final_router_price = Router::spot_price_with_fee(&route).unwrap(); + assert!(otc_price > final_router_price); + assert!(final_router_price > initial_router_price); + + assert_eq!(hdx_total_issuance, Currencies::total_issuance(HDX)); + assert_eq!(dai_total_issuance, Currencies::total_issuance(DAI)); + + // total issuance of tokens should not change + assert!(Currencies::free_balance(HDX, &OtcSettlements::account_id()) == 0); + assert!(Currencies::free_balance(DAI, &OtcSettlements::account_id()) == 0); + + expect_last_events(vec![Event::Executed { + asset_id: HDX, + profit: 1_444_117_874_415, + } + .into()]); + }); +} + #[test] fn existing_arb_opportunity_should_trigger_trade_when_otc_is_not_partially_fillable() { let (mut ext, _) = ExtBuilder::default().build(); @@ -562,7 +618,7 @@ fn trade_should_not_be_triggered_when_there_is_no_arb_opportunity() { } #[test] -fn trade_should_not_be_triggered_when_optimal_amount_not_found() { +fn trade_should_be_triggered_when_optimal_amount_not_found_but_arb_can_be_reduced() { let (mut ext, _) = ExtBuilder::default().build(); ext.execute_with(|| { assert_ok!(OTC::place_order( @@ -595,9 +651,7 @@ fn trade_should_not_be_triggered_when_optimal_amount_not_found() { assert!(Currencies::free_balance(HDX, &OtcSettlements::account_id()) == 0); assert!(Currencies::free_balance(DAI, &OtcSettlements::account_id()) == 0); - assert_storage_noop!(>>::offchain_worker( - System::block_number() - )); + >>::offchain_worker(System::block_number()); assert_eq!(hdx_total_issuance, Currencies::total_issuance(HDX)); assert_eq!(dai_total_issuance, Currencies::total_issuance(DAI)); @@ -608,6 +662,43 @@ fn trade_should_not_be_triggered_when_optimal_amount_not_found() { }); } +#[test] +fn trade_should_not_be_triggered_when_amount_not_found() { + let (mut ext, _) = ExtBuilder::default().build(); + ext.execute_with(|| { + assert_ok!(OTC::place_order( + RuntimeOrigin::signed(ALICE), + HDX, // otc asset_in + DAI, // otc asset_out + 1_000 * ONE, + 800_000_000_000_000_001 * ONE, + true, + )); + + // get otc price + let otc_id = 0; + let otc = >::get(otc_id).unwrap(); + let otc_price = calculate_otc_price(&otc); + + // get trade price + let route = Router::get_route(AssetPair { + asset_in: otc.asset_out, + asset_out: otc.asset_in, + }); + let router_price = Router::spot_price_with_fee(&route).unwrap(); + + // verify that there's an arb opportunity + assert!(otc_price > router_price); + + assert!(Currencies::free_balance(HDX, &OtcSettlements::account_id()) == 0); + assert!(Currencies::free_balance(DAI, &OtcSettlements::account_id()) == 0); + + assert_storage_noop!(>>::offchain_worker( + System::block_number() + )); + }); +} + #[test] fn test_offchain_worker_unsigned_transaction_submission() { let (mut ext, pool_state) = ExtBuilder::default().build(); diff --git a/runtime/hydradx/Cargo.toml b/runtime/hydradx/Cargo.toml index 0b38f82bf..60e6e1458 100644 --- a/runtime/hydradx/Cargo.toml +++ b/runtime/hydradx/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "hydradx-runtime" -version = "254.0.0" +version = "255.0.0" authors = ["GalacticCouncil"] edition = "2021" license = "Apache 2.0" diff --git a/runtime/hydradx/src/lib.rs b/runtime/hydradx/src/lib.rs index f7d4e1b7e..e80d28be2 100644 --- a/runtime/hydradx/src/lib.rs +++ b/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: 254, + spec_version: 255, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1,